Safer serialization of base types (#18)

* use MESSAGE_SIZE constant instead of 32
* use Hashed trait in genesis
* safer serialization of Hash
* Hashed of [u8] should send only data to hash function, not length + data
* Safer serialization of Proof
* Safer serialization of Commitment
* Safer serialization of RangeProof
* introduce read_limited_vec instead of potential panic in conversion

chain/src/types.rs

@@ -111,13 +111,13 @@ impl ser::Writeable for Tip {
 impl ser::Readable<Tip> for Tip {
 	fn read(reader: &mut ser::Reader) -> Result<Tip, ser::Error> {
 		let height = try!(reader.read_u64());
-		let last = try!(reader.read_fixed_bytes(32));
+		let last = try!(Hash::read(reader));
-		let prev = try!(reader.read_fixed_bytes(32));
+		let prev = try!(Hash::read(reader));
 		let line = try!(Lineage::read(reader));
 		Ok(Tip {
 			height: height,
-			last_block_h: Hash::from_vec(last),
+			last_block_h: last,
-			prev_block_h: Hash::from_vec(prev),
+			prev_block_h: prev,
 			lineage: line,
 		})
 	}

core/src/core/block.rs

@@ -23,7 +23,7 @@ use std::collections::HashSet;
 use core::Committed;
 use core::{Input, Output, Proof, TxProof, Transaction};
 use core::transaction::merkle_inputs_outputs;
-use consensus::{PROOFSIZE, REWARD, DEFAULT_SIZESHIFT, MAX_TARGET};
+use consensus::{REWARD, DEFAULT_SIZESHIFT, MAX_TARGET};
 use core::hash::{Hash, Hashed, ZERO_HASH};
 use core::target::Target;
 use ser::{self, Readable, Reader, Writeable, Writer};
@@ -79,40 +79,35 @@ impl Writeable for BlockHeader {
 		// make sure to not introduce any variable length data before the nonce to
 		// avoid complicating PoW
 		try!(writer.write_u64(self.nonce));
-		// cuckoo cycle of 42 nodes
+		// proof
-		for n in 0..PROOFSIZE {
+		self.pow.write(writer)
-			try!(writer.write_u32(self.pow.0[n]));
-		}
-		Ok(())
 	}
 }
 
 /// Deserialization of a block header
 impl Readable<BlockHeader> for BlockHeader {
 	fn read(reader: &mut Reader) -> Result<BlockHeader, ser::Error> {
-		let (height, previous, timestamp, cuckoo_len) =
+		let height = try!(reader.read_u64());
-			ser_multiread!(reader, read_u64, read_32_bytes, read_i64, read_u8);
+		let previous = try!(Hash::read(reader));
+		let (timestamp, cuckoo_len) = ser_multiread!(reader, read_i64, read_u8);
 		let target = try!(Target::read(reader));
-		let (utxo_merkle, tx_merkle, nonce) =
+		let utxo_merkle = try!(Hash::read(reader));
-			ser_multiread!(reader, read_32_bytes, read_32_bytes, read_u64);
+		let tx_merkle = try!(Hash::read(reader));
+		let nonce = try!(reader.read_u64());
+		let pow = try!(Proof::read(reader));
 
-		// cuckoo cycle of 42 nodes
-		let mut pow = [0; PROOFSIZE];
-		for n in 0..PROOFSIZE {
-			pow[n] = try!(reader.read_u32());
-		}
 		Ok(BlockHeader {
 			height: height,
-			previous: Hash::from_vec(previous),
+			previous: previous,
 			timestamp: time::at_utc(time::Timespec {
 				sec: timestamp,
 				nsec: 0,
 			}),
 			cuckoo_len: cuckoo_len,
 			target: target,
-			utxo_merkle: Hash::from_vec(utxo_merkle),
+			utxo_merkle: utxo_merkle,
-			tx_merkle: Hash::from_vec(tx_merkle),
+			tx_merkle: tx_merkle,
-			pow: Proof(pow),
+			pow: pow,
 			nonce: nonce,
 		})
 	}
@@ -356,7 +351,7 @@ impl Block {
 	fn reward_output(skey: secp::key::SecretKey,
 	                 secp: &Secp256k1)
 	                 -> Result<(Output, TxProof), secp::Error> {
-		let msg = try!(secp::Message::from_slice(&[0; 32]));
+		let msg = try!(secp::Message::from_slice(&[0; secp::constants::MESSAGE_SIZE]));
 		let sig = try!(secp.sign(&msg, &skey));
 		let output = Output::OvertOutput {
 				value: REWARD,

core/src/core/hash.rs

@@ -21,7 +21,7 @@ use byteorder::{ByteOrder, BigEndian};
 use std::fmt;
 use tiny_keccak::Keccak;
 
-use ser::{self, AsFixedBytes};
+use ser::{self, AsFixedBytes, Reader, Readable, Error};
 
 pub const ZERO_HASH: Hash = Hash([0; 32]);
 
@@ -40,14 +40,6 @@ impl fmt::Display for Hash {
 }
 
 impl Hash {
-	/// Creates a new hash from a vector
-	pub fn from_vec(v: Vec<u8>) -> Hash {
-		let mut a = [0; 32];
-		for i in 0..a.len() {
-			a[i] = v[i];
-		}
-		Hash(a)
-	}
 	/// Converts the hash to a byte vector
 	pub fn to_vec(&self) -> Vec<u8> {
 		self.0.to_vec()
@@ -58,6 +50,18 @@ impl Hash {
 	}
 }
 
+
+impl Readable<Hash> for Hash {
+	fn read(reader: &mut Reader) -> Result<Hash, ser::Error> {
+		let v = try!(reader.read_fixed_bytes(32));
+		let mut a = [0; 32];
+		for i in 0..a.len() {
+			a[i] = v[i];
+		}
+		Ok(Hash(a))
+	}
+}
+
 /// Serializer that outputs a hash of the serialized object
 pub struct HashWriter {
 	state: Keccak,
@@ -105,7 +109,7 @@ impl Hashed for [u8] {
 	fn hash(&self) -> Hash {
 		let mut hasher = HashWriter::default();
 		let mut ret = [0; 32];
-		ser::Writer::write_bytes(&mut hasher, &self).unwrap();
+		ser::Writer::write_fixed_bytes(&mut hasher, &self).unwrap();
 		hasher.finalize(&mut ret);
 		Hash(ret)
 	}

core/src/core/mod.rs

@@ -30,7 +30,7 @@ use consensus::PROOFSIZE;
 pub use self::block::{Block, BlockHeader};
 pub use self::transaction::{Transaction, Input, Output, TxProof};
 use self::hash::{Hash, Hashed, HashWriter, ZERO_HASH};
-use ser::{Writeable, Writer, Error};
+use ser::{Writeable, Writer, Reader, Readable, Error};
 
 /// Implemented by types that hold inputs and outputs including Pedersen
 /// commitments. Handles the collection of the commitments as well as their
@@ -109,32 +109,10 @@ impl Clone for Proof {
 	}
 }
 
-impl Hashed for Proof {
-	fn hash(&self) -> Hash {
-		let mut hasher = HashWriter::default();
-		let mut ret = [0; 32];
-		for p in self.0.iter() {
-			hasher.write_u32(*p).unwrap();
-		}
-		hasher.finalize(&mut ret);
-		Hash(ret)
-	}
-}
-
 impl Proof {
 	/// Builds a proof with all bytes zeroed out
 	pub fn zero() -> Proof {
-		Proof([0; 42])
+		Proof([0; PROOFSIZE])
-	}
-
-	/// Builds a proof from a vector of exactly PROOFSIZE (42) u32.
-	pub fn from_vec(v: Vec<u32>) -> Proof {
-		assert!(v.len() == PROOFSIZE);
-		let mut p = [0; PROOFSIZE];
-		for (n, elem) in v.iter().enumerate() {
-			p[n] = *elem;
-		}
-		Proof(p)
 	}
 
 	/// Converts the proof to a vector of u64s
@@ -158,6 +136,25 @@ impl Proof {
 	}
 }
 
+impl Readable<Proof> for Proof {
+	fn read(reader: &mut Reader) -> Result<Proof, Error> {
+		let mut pow = [0u32; PROOFSIZE];
+		for n in 0..PROOFSIZE {
+			pow[n] = try!(reader.read_u32());
+		}
+		Ok(Proof(pow))
+	}
+}
+
+impl Writeable for Proof {
+	fn write(&self, writer: &mut Writer) -> Result<(), Error> {
+		for n in 0..PROOFSIZE {
+			try!(writer.write_u32(self.0[n]));
+		}
+		Ok(())
+	}
+}
+
 /// Two hashes that will get hashed together in a Merkle tree to build the next
 /// level up.
 struct HPair(Hash, Hash);

core/src/core/transaction.rs

@@ -51,9 +51,10 @@ impl Writeable for TxProof {
 
 impl Readable<TxProof> for TxProof {
 	fn read(reader: &mut Reader) -> Result<TxProof, ser::Error> {
-		let (remainder, sig, fee) = ser_multiread!(reader, read_33_bytes, read_vec, read_u64);
+		let remainder = try!(Commitment::read (reader));
+		let (sig, fee) = ser_multiread!(reader, read_vec, read_u64);
 		Ok(TxProof {
-			remainder: Commitment::from_vec(remainder),
+			remainder: remainder,
 			sig: sig,
 			fee: fee,
 		})
@@ -257,8 +258,8 @@ impl Writeable for Input {
 /// an Input from a binary stream.
 impl Readable<Input> for Input {
 	fn read(reader: &mut Reader) -> Result<Input, ser::Error> {
-		reader.read_fixed_bytes(32)
+		Hash::read(reader)
-			.map(|h| Input::BareInput { output: Hash::from_vec(h) })
+			.map(|h| Input::BareInput { output: h })
 	}
 }
 
@@ -322,10 +323,11 @@ impl Writeable for Output {
 /// an Output from a binary stream.
 impl Readable<Output> for Output {
 	fn read(reader: &mut Reader) -> Result<Output, ser::Error> {
-		let (commit, proof) = ser_multiread!(reader, read_33_bytes, read_vec);
+		let commit = try!(Commitment::read(reader));
+		let proof = try!(RangeProof::read(reader));
 		Ok(Output::BlindOutput {
-			commit: Commitment::from_vec(commit),
+			commit: commit,
-			proof: RangeProof::from_vec(proof),
+			proof: proof,
 		})
 	}
 }

core/src/genesis.rs

@@ -18,17 +18,11 @@ use time;
 
 use core;
 use consensus::{DEFAULT_SIZESHIFT, MAX_TARGET};
-
+use core::hash::Hashed;
-use tiny_keccak::Keccak;
 
 // Genesis block definition. It has no rewards, no inputs, no outputs, no
 // fees and a height of zero.
 pub fn genesis() -> core::Block {
-	let mut sha3 = Keccak::new_sha3_256();
-	let mut empty_h = [0; 32];
-	sha3.update(&[]);
-	sha3.finalize(&mut empty_h);
-
 	core::Block {
 		header: core::BlockHeader {
 			height: 0,
@@ -41,8 +35,8 @@ pub fn genesis() -> core::Block {
 			},
 			cuckoo_len: DEFAULT_SIZESHIFT,
 			target: MAX_TARGET,
-			utxo_merkle: core::hash::Hash::from_vec(empty_h.to_vec()),
+			utxo_merkle:  [].hash(),
-			tx_merkle: core::hash::Hash::from_vec(empty_h.to_vec()),
+			tx_merkle:  [].hash(),
 			nonce: 0,
 			pow: core::Proof::zero(), // TODO get actual PoW solution
 		},

core/src/ser.rs

@@ -19,9 +19,13 @@
 //! To use it simply implement `Writeable` or `Readable` and then use the
 //! `serialize` or `deserialize` functions on them as appropriate.
 
-use std::{error, fmt};
+use std::{error, fmt, cmp};
 use std::io::{self, Write, Read};
 use byteorder::{ByteOrder, ReadBytesExt, BigEndian};
+use secp::pedersen::Commitment;
+use secp::pedersen::RangeProof;
+use secp::constants::PEDERSEN_COMMITMENT_SIZE;
+use secp::constants::MAX_PROOF_SIZE;
 
 /// Possible errors deriving from serializing or deserializing.
 #[derive(Debug)]
@@ -161,12 +165,10 @@ pub trait Reader {
 	fn read_i64(&mut self) -> Result<i64, Error>;
 	/// first before the data bytes.
 	fn read_vec(&mut self) -> Result<Vec<u8>, Error>;
+	/// first before the data bytes limited to max bytes.
+	fn read_limited_vec(&mut self, max: usize) -> Result<Vec<u8>, Error>;
 	/// Read a fixed number of bytes from the underlying reader.
 	fn read_fixed_bytes(&mut self, length: usize) -> Result<Vec<u8>, Error>;
-	/// Convenience function to read 32 fixed bytes
-	fn read_32_bytes(&mut self) -> Result<Vec<u8>, Error>;
-	/// Convenience function to read 33 fixed bytes
-	fn read_33_bytes(&mut self) -> Result<Vec<u8>, Error>;
 	/// Consumes a byte from the reader, producing an error if it doesn't have
 	/// the expected value
 	fn expect_u8(&mut self, val: u8) -> Result<u8, Error>;
@@ -235,6 +237,11 @@ impl<'a> Reader for BinReader<'a> {
 		let len = try!(self.read_u64());
 		self.read_fixed_bytes(len as usize)
 	}
+	/// Read limited variable size vector from the underlying Read. Expects a usize
+	fn read_limited_vec(&mut self, max: usize) -> Result<Vec<u8>, Error> {
+		let len = cmp::min(max, try!(self.read_u64()) as usize);
+		self.read_fixed_bytes(len as usize)
+	}
 	fn read_fixed_bytes(&mut self, length: usize) -> Result<Vec<u8>, Error> {
 		// not reading more than 100k in a single read
 		if length > 100000 {
@@ -243,12 +250,7 @@ impl<'a> Reader for BinReader<'a> {
 		let mut buf = vec![0; length];
 		self.source.read_exact(&mut buf).map(move |_| buf).map_err(Error::IOErr)
 	}
-	fn read_32_bytes(&mut self) -> Result<Vec<u8>, Error> {
+
-		self.read_fixed_bytes(32)
-	}
-	fn read_33_bytes(&mut self) -> Result<Vec<u8>, Error> {
-		self.read_fixed_bytes(33)
-	}
 	fn expect_u8(&mut self, val: u8) -> Result<u8, Error> {
 		let b = try!(self.read_u8());
 		if b == val {
@@ -262,6 +264,32 @@ impl<'a> Reader for BinReader<'a> {
 	}
 }
 
+
+impl Readable<Commitment> for Commitment {
+	fn read(reader: &mut Reader) -> Result<Commitment, Error> {
+		let a = try!(reader.read_fixed_bytes(PEDERSEN_COMMITMENT_SIZE));
+		let mut c = [0; PEDERSEN_COMMITMENT_SIZE];
+		for i in 0..PEDERSEN_COMMITMENT_SIZE {
+			c[i] = a[i];
+		}
+		Ok(Commitment(c))
+	}
+}
+
+impl Readable<RangeProof> for RangeProof {
+	fn read(reader: &mut Reader) -> Result<RangeProof, Error> {
+		let p = try!(reader.read_limited_vec(MAX_PROOF_SIZE));
+		let mut a = [0; MAX_PROOF_SIZE];
+		for i in 0..p.len() {
+			a[i] = p[i];
+		}
+		Ok(RangeProof {
+			proof: a,
+			plen: p.len(),
+		})
+	}
+}
+
 /// Utility wrapper for an underlying byte Writer. Defines higher level methods
 /// to write numbers, byte vectors, hashes, etc.
 struct BinWriter<'a> {

secp256k1zkp/src/pedersen.rs

@@ -28,7 +28,7 @@ use key::{SecretKey, PublicKey};
 use rand::{Rng, OsRng};
 
 /// A Pedersen commitment
-pub struct Commitment([u8; constants::PEDERSEN_COMMITMENT_SIZE]);
+pub struct Commitment(pub [u8; constants::PEDERSEN_COMMITMENT_SIZE]);
 impl_array_newtype!(Commitment, u8, constants::PEDERSEN_COMMITMENT_SIZE);
 impl_pretty_debug!(Commitment);
 
@@ -37,14 +37,6 @@ impl Commitment {
     unsafe fn blank() -> Commitment {
         mem::uninitialized()
     }
-    /// Builds a commitment from a binary vector. Will panic if the vector is too short.
-    pub fn from_vec(c: Vec<u8>) -> Commitment {
-        let mut a = [0; constants::PEDERSEN_COMMITMENT_SIZE];
-        for i in 0..a.len() {
-            a[i] = c[i];
-        }
-        Commitment(a)
-    }
     /// Converts a commitment to a public key
     pub fn to_pubkey(&self, secp: &Secp256k1) -> Result<key::PublicKey, Error> {
         key::PublicKey::from_slice(secp, &self.0)
@@ -83,17 +75,6 @@ impl Clone for RangeProof {
 }
 
 impl RangeProof {
-    /// Builds a range proof from a binary vector. Will panic if the vector is longer than the max proof size.
-    pub fn from_vec(p: Vec<u8>) -> RangeProof {
-        let mut a = [0; constants::MAX_PROOF_SIZE];
-        for i in 0..p.len() {
-            a[i] = p[i];
-        }
-        RangeProof {
-            proof: a,
-            plen: p.len(),
-        }
-    }
     pub fn bytes(&self) -> &[u8] {
         &self.proof[..self.plen as usize]
     }