From bc380163855cac465bf19c003709c46d048566c1 Mon Sep 17 00:00:00 2001 From: Merope Riddle Date: Mon, 31 Oct 2016 22:17:53 +0000 Subject: [PATCH 1/6] core/ser: change serialization trait to use Result<(), Error> in place of Option --- core/src/core/block.rs | 18 +++---- core/src/core/transaction.rs | 30 +++++------ core/src/macros.rs | 15 +----- core/src/pow/mod.rs | 20 +++---- core/src/ser.rs | 102 ++++++++++++++++++++++++----------- 5 files changed, 104 insertions(+), 81 deletions(-) diff --git a/core/src/core/block.rs b/core/src/core/block.rs index e18013171..5d3ee716e 100644 --- a/core/src/core/block.rs +++ b/core/src/core/block.rs @@ -60,7 +60,7 @@ impl Default for BlockHeader { // Only Writeable implementation is required for hashing, which is part of // core. Readable is in the ser package. impl Writeable for BlockHeader { - fn write(&self, writer: &mut Writer) -> Option { + fn write(&self, writer: &mut Writer) -> Result<(), ser::Error> { ser_multiwrite!(writer, [write_u64, self.height], [write_fixed_bytes, &self.previous], @@ -70,10 +70,10 @@ impl Writeable for BlockHeader { [write_u64, self.total_fees]); // make sure to not introduce any variable length data before the nonce to // avoid complicating PoW - try_o!(writer.write_u64(self.nonce)); + try!(writer.write_u64(self.nonce)); // cuckoo cycle of 42 nodes for n in 0..42 { - try_o!(writer.write_u32(self.pow.0[n])); + try!(writer.write_u32(self.pow.0[n])); } writer.write_u64(self.td) } @@ -101,23 +101,23 @@ pub struct Block { /// Implementation of Writeable for a block, defines how to write the full /// block as binary. impl Writeable for Block { - fn write(&self, writer: &mut Writer) -> Option { - try_o!(self.header.write(writer)); + fn write(&self, writer: &mut Writer) -> Result<(), ser::Error> { + try!(self.header.write(writer)); ser_multiwrite!(writer, [write_u64, self.inputs.len() as u64], [write_u64, self.outputs.len() as u64], [write_u64, self.proofs.len() as u64]); for inp in &self.inputs { - try_o!(inp.write(writer)); + try!(inp.write(writer)); } for out in &self.outputs { - try_o!(out.write(writer)); + try!(out.write(writer)); } for proof in &self.proofs { - try_o!(proof.write(writer)); + try!(proof.write(writer)); } - None + Ok(()) } } diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index 98566c83a..a250cb90e 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -39,8 +39,8 @@ pub struct TxProof { } impl Writeable for TxProof { - fn write(&self, writer: &mut Writer) -> Option { - try_o!(writer.write_fixed_bytes(&self.remainder)); + fn write(&self, writer: &mut Writer) -> Result<(), ser::Error> { + try!(writer.write_fixed_bytes(&self.remainder)); writer.write_vec(&mut self.sig.clone()) } } @@ -68,19 +68,19 @@ pub struct Transaction { /// Implementation of Writeable for a fully blinded transaction, defines how to /// write the transaction as binary. impl Writeable for Transaction { - fn write(&self, writer: &mut Writer) -> Option { + fn write(&self, writer: &mut Writer) -> Result<(), ser::Error> { ser_multiwrite!(writer, [write_u64, self.fee], [write_vec, &mut self.zerosig.clone()], [write_u64, self.inputs.len() as u64], [write_u64, self.outputs.len() as u64]); for inp in &self.inputs { - try_o!(inp.write(writer)); + try!(inp.write(writer)); } for out in &self.outputs { - try_o!(out.write(writer)); + try!(out.write(writer)); } - None + Ok(()) } } @@ -235,7 +235,7 @@ pub enum Input { /// Implementation of Writeable for a transaction Input, defines how to write /// an Input as binary. impl Writeable for Input { - fn write(&self, writer: &mut Writer) -> Option { + fn write(&self, writer: &mut Writer) -> Result<(), ser::Error> { writer.write_fixed_bytes(&self.output_hash()) } } @@ -302,8 +302,8 @@ pub enum Output { /// Implementation of Writeable for a transaction Output, defines how to write /// an Output as binary. impl Writeable for Output { - fn write(&self, writer: &mut Writer) -> Option { - try_o!(writer.write_fixed_bytes(&self.commitment().unwrap())); + fn write(&self, writer: &mut Writer) -> Result<(), ser::Error> { + try!(writer.write_fixed_bytes(&self.commitment().unwrap())); writer.write_vec(&mut self.proof().unwrap().bytes().to_vec()) } } @@ -407,9 +407,7 @@ mod test { let tx = tx2i1o(secp, &mut rng); let btx = tx.blind(&secp).unwrap(); let mut vec = Vec::new(); - if let Some(e) = serialize(&mut vec, &btx) { - panic!(e); - } + serialize(&mut vec, &btx).expect("serialized failed"); assert!(vec.len() > 5320); assert!(vec.len() < 5340); } @@ -422,9 +420,7 @@ mod test { let tx = tx2i1o(secp, &mut rng); let mut btx = tx.blind(&secp).unwrap(); let mut vec = Vec::new(); - if let Some(e) = serialize(&mut vec, &btx) { - panic!(e); - } + serialize(&mut vec, &btx).expect("serialization failed"); // let mut dtx = Transaction::read(&mut BinReader { source: &mut &vec[..] // }).unwrap(); let mut dtx: Transaction = deserialize(&mut &vec[..]).unwrap(); @@ -444,11 +440,11 @@ mod test { let mut btx = tx.blind(&secp).unwrap(); let mut vec = Vec::new(); - assert!(serialize(&mut vec, &btx).is_none()); + assert!(serialize(&mut vec, &btx).is_ok()); let mut dtx: Transaction = deserialize(&mut &vec[..]).unwrap(); let mut vec2 = Vec::new(); - assert!(serialize(&mut vec2, &btx).is_none()); + assert!(serialize(&mut vec2, &btx).is_ok()); let mut dtx2: Transaction = deserialize(&mut &vec2[..]).unwrap(); assert_eq!(btx.hash(), dtx.hash()); diff --git a/core/src/macros.rs b/core/src/macros.rs index 59d3017e4..0f13a999e 100644 --- a/core/src/macros.rs +++ b/core/src/macros.rs @@ -61,19 +61,6 @@ macro_rules! tee { } } -/// Simple equivalent of try! but for an Option. Motivated mostly by the -/// io package and our serialization as an alternative to silly Result<(), -/// Error>. -#[macro_export] -macro_rules! try_o { - ($trying:expr) => { - let tried = $trying; - if let Some(_) = tried { - return tried; - } - } -} - #[macro_export] macro_rules! try_to_o { ($trying:expr) => {{ @@ -109,6 +96,6 @@ macro_rules! ser_multiread { #[macro_export] macro_rules! ser_multiwrite { ($wrtr:ident, $([ $write_call:ident, $val:expr ]),* ) => { - $( try_o!($wrtr.$write_call($val)) );* + $( try!($wrtr.$write_call($val)) );* } } diff --git a/core/src/pow/mod.rs b/core/src/pow/mod.rs index b06383aea..d17da511b 100644 --- a/core/src/pow/mod.rs +++ b/core/src/pow/mod.rs @@ -71,16 +71,16 @@ struct PowHeader { /// the data that gets hashed for PoW calculation. The nonce is written first /// to make incrementing from the serialized form trivial. impl Writeable for PowHeader { - fn write(&self, writer: &mut Writer) -> Option { - try_o!(writer.write_u64(self.nonce)); - try_o!(writer.write_u64(self.height)); - try_o!(writer.write_fixed_bytes(&self.previous)); - try_o!(writer.write_i64(self.timestamp.to_timespec().sec)); - try_o!(writer.write_fixed_bytes(&self.utxo_merkle)); - try_o!(writer.write_fixed_bytes(&self.tx_merkle)); - try_o!(writer.write_u64(self.total_fees)); - try_o!(writer.write_u64(self.n_in)); - try_o!(writer.write_u64(self.n_out)); + fn write(&self, writer: &mut Writer) -> Result<(), ser::Error> { + try!(writer.write_u64(self.nonce)); + try!(writer.write_u64(self.height)); + try!(writer.write_fixed_bytes(&self.previous)); + try!(writer.write_i64(self.timestamp.to_timespec().sec)); + try!(writer.write_fixed_bytes(&self.utxo_merkle)); + try!(writer.write_fixed_bytes(&self.tx_merkle)); + try!(writer.write_u64(self.total_fees)); + try!(writer.write_u64(self.n_in)); + try!(writer.write_u64(self.n_out)); writer.write_u64(self.n_proofs) } } diff --git a/core/src/ser.rs b/core/src/ser.rs index 99680c5f7..e1f5e4e3e 100644 --- a/core/src/ser.rs +++ b/core/src/ser.rs @@ -19,8 +19,8 @@ //! To use it simply implement `Writeable` or `Readable` and then use the //! `serialize` or `deserialize` functions on them as appropriate. -use std::io; -use std::io::{Write, Read}; +use std::{error, fmt}; +use std::io::{self, Write, Read}; use byteorder::{ReadBytesExt, WriteBytesExt, BigEndian}; /// Possible errors deriving from serializing or deserializing. @@ -33,12 +33,47 @@ pub enum Error { expected: Vec, received: Vec, }, - /// Data wasn't in a consumable format - CorruptedData, + /// Data wasn't in a consumable format + CorruptedData, /// When asked to read too much data TooLargeReadErr(String), } +impl From for Error { + fn from(e: io::Error) -> Error { + Error::IOErr(e) + } +} + +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match *self { + Error::IOErr(ref e) => write!(f, "{}", e), + Error::UnexpectedData { expected: ref e, received: ref r } => write!(f, "expected {:?}, got {:?}", e, r), + Error::CorruptedData => f.write_str("corrupted data"), + Error::TooLargeReadErr(ref s) => f.write_str(&s) + } + } +} + +impl error::Error for Error { + fn cause(&self) -> Option<&error::Error> { + match *self { + Error::IOErr(ref e) => Some(e), + _ => None + } + } + + fn description(&self) -> &str { + match *self { + Error::IOErr(ref e) => error::Error::description(e), + Error::UnexpectedData { expected: _, received: _ } => "unexpected data", + Error::CorruptedData => "corrupted data", + Error::TooLargeReadErr(ref s) => s + } + } +} + /// Useful trait to implement on types that can be translated to byte slices /// directly. Allows the use of `write_fixed_bytes` on them. pub trait AsFixedBytes { @@ -50,21 +85,21 @@ pub trait AsFixedBytes { /// written to an underlying stream or container (depending on implementation). pub trait Writer { /// Writes a u8 as bytes - fn write_u8(&mut self, n: u8) -> Option; + fn write_u8(&mut self, n: u8) -> Result<(), Error>; /// Writes a u16 as bytes - fn write_u16(&mut self, n: u16) -> Option; + fn write_u16(&mut self, n: u16) -> Result<(), Error>; /// Writes a u32 as bytes - fn write_u32(&mut self, n: u32) -> Option; + fn write_u32(&mut self, n: u32) -> Result<(), Error>; /// Writes a u64 as bytes - fn write_u64(&mut self, n: u64) -> Option; + fn write_u64(&mut self, n: u64) -> Result<(), Error>; /// Writes a i64 as bytes - fn write_i64(&mut self, n: i64) -> Option; + fn write_i64(&mut self, n: i64) -> Result<(), Error>; /// Writes a variable length `Vec`, the length of the `Vec` is encoded as a /// prefix. - fn write_vec(&mut self, vec: &mut Vec) -> Option; + fn write_vec(&mut self, vec: &mut Vec) -> Result<(), Error>; /// Writes a fixed number of bytes from something that can turn itself into /// a `&[u8]`. The reader is expected to know the actual length on read. - fn write_fixed_bytes(&mut self, b32: &AsFixedBytes) -> Option; + fn write_fixed_bytes(&mut self, b32: &AsFixedBytes) -> Result<(), Error>; } /// Implementations defined how different numbers and binary structures are @@ -98,7 +133,7 @@ pub trait Reader { /// underlying Write implementation. pub trait Writeable { /// Write the data held by this Writeable to the provided writer - fn write(&self, writer: &mut Writer) -> Option; + fn write(&self, writer: &mut Writer) -> Result<(), Error>; } /// Trait that every type that can be deserialized from binary must implement. @@ -116,7 +151,7 @@ pub fn deserialize>(mut source: &mut Read) -> Result { } /// Serializes a Writeable into any std::io::Write implementation. -pub fn serialize(mut sink: &mut Write, thing: &Writeable) -> Option { +pub fn serialize(mut sink: &mut Write, thing: &Writeable) -> Result<(), Error> { let mut writer = BinWriter { sink: sink }; thing.write(&mut writer) } @@ -125,9 +160,7 @@ pub fn serialize(mut sink: &mut Write, thing: &Writeable) -> Option { /// Vec. pub fn ser_vec(thing: &Writeable) -> Result, Error> { let mut vec = Vec::new(); - if let Some(err) = serialize(&mut vec, thing) { - return Err(err); - } + try!(serialize(&mut vec, thing)); Ok(vec) } @@ -192,33 +225,40 @@ struct BinWriter<'a> { } impl<'a> Writer for BinWriter<'a> { - fn write_u8(&mut self, n: u8) -> Option { - self.sink.write_u8(n).err().map(Error::IOErr) + fn write_u8(&mut self, n: u8) -> Result<(), Error> { + try!(self.sink.write_u8(n)); + Ok(()) } - fn write_u16(&mut self, n: u16) -> Option { - self.sink.write_u16::(n).err().map(Error::IOErr) + fn write_u16(&mut self, n: u16) -> Result<(), Error> { + try!(self.sink.write_u16::(n)); + Ok(()) } - fn write_u32(&mut self, n: u32) -> Option { - self.sink.write_u32::(n).err().map(Error::IOErr) + fn write_u32(&mut self, n: u32) -> Result<(), Error> { + try!(self.sink.write_u32::(n)); + Ok(()) } - fn write_u64(&mut self, n: u64) -> Option { - self.sink.write_u64::(n).err().map(Error::IOErr) + fn write_u64(&mut self, n: u64) -> Result<(), Error> { + try!(self.sink.write_u64::(n)); + Ok(()) } - fn write_i64(&mut self, n: i64) -> Option { - self.sink.write_i64::(n).err().map(Error::IOErr) + fn write_i64(&mut self, n: i64) -> Result<(), Error> { + try!(self.sink.write_i64::(n)); + Ok(()) } - fn write_vec(&mut self, vec: &mut Vec) -> Option { - try_o!(self.write_u64(vec.len() as u64)); - self.sink.write_all(vec).err().map(Error::IOErr) + fn write_vec(&mut self, vec: &mut Vec) -> Result<(), Error> { + try!(self.write_u64(vec.len() as u64)); + try!(self.sink.write_all(vec)); + Ok(()) } - fn write_fixed_bytes(&mut self, b32: &AsFixedBytes) -> Option { + fn write_fixed_bytes(&mut self, b32: &AsFixedBytes) -> Result<(), Error> { let bs = b32.as_fixed_bytes(); - self.sink.write_all(bs).err().map(Error::IOErr) + try!(self.sink.write_all(bs)); + Ok(()) } } From 630c4eb6fb7b8d2b04809c2fd5ded06d2e53a87e Mon Sep 17 00:00:00 2001 From: Merope Riddle Date: Mon, 31 Oct 2016 22:24:43 +0000 Subject: [PATCH 2/6] core/ser: replace `write_vec` with `write_bytes`, drop a bunch of clones --- core/src/core/transaction.rs | 6 +++--- core/src/ser.rs | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index a250cb90e..5af9ccd79 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -41,7 +41,7 @@ pub struct TxProof { impl Writeable for TxProof { fn write(&self, writer: &mut Writer) -> Result<(), ser::Error> { try!(writer.write_fixed_bytes(&self.remainder)); - writer.write_vec(&mut self.sig.clone()) + writer.write_bytes(&self.sig) } } @@ -71,7 +71,7 @@ impl Writeable for Transaction { fn write(&self, writer: &mut Writer) -> Result<(), ser::Error> { ser_multiwrite!(writer, [write_u64, self.fee], - [write_vec, &mut self.zerosig.clone()], + [write_bytes, &self.zerosig], [write_u64, self.inputs.len() as u64], [write_u64, self.outputs.len() as u64]); for inp in &self.inputs { @@ -304,7 +304,7 @@ pub enum Output { impl Writeable for Output { fn write(&self, writer: &mut Writer) -> Result<(), ser::Error> { try!(writer.write_fixed_bytes(&self.commitment().unwrap())); - writer.write_vec(&mut self.proof().unwrap().bytes().to_vec()) + writer.write_bytes(&mut self.proof().unwrap().bytes()) } } diff --git a/core/src/ser.rs b/core/src/ser.rs index e1f5e4e3e..a0e09c7a3 100644 --- a/core/src/ser.rs +++ b/core/src/ser.rs @@ -96,7 +96,7 @@ pub trait Writer { fn write_i64(&mut self, n: i64) -> Result<(), Error>; /// Writes a variable length `Vec`, the length of the `Vec` is encoded as a /// prefix. - fn write_vec(&mut self, vec: &mut Vec) -> Result<(), Error>; + fn write_bytes(&mut self, data: &[u8]) -> Result<(), Error>; /// Writes a fixed number of bytes from something that can turn itself into /// a `&[u8]`. The reader is expected to know the actual length on read. fn write_fixed_bytes(&mut self, b32: &AsFixedBytes) -> Result<(), Error>; @@ -249,9 +249,9 @@ impl<'a> Writer for BinWriter<'a> { } - fn write_vec(&mut self, vec: &mut Vec) -> Result<(), Error> { - try!(self.write_u64(vec.len() as u64)); - try!(self.sink.write_all(vec)); + fn write_bytes(&mut self, data: &[u8]) -> Result<(), Error> { + try!(self.write_u64(data.len() as u64)); + try!(self.sink.write_all(data)); Ok(()) } From 565374bac7dc4089c2f3d91fb7979e901ad885a6 Mon Sep 17 00:00:00 2001 From: Merope Riddle Date: Tue, 1 Nov 2016 02:03:12 +0000 Subject: [PATCH 3/6] p2p: fix for changes in core serialization API --- p2p/src/handshake.rs | 38 +++++++++++++++----------------------- p2p/src/msg.rs | 32 ++++++++++++++++---------------- 2 files changed, 31 insertions(+), 39 deletions(-) diff --git a/p2p/src/handshake.rs b/p2p/src/handshake.rs index 24305c8b4..0534d03e6 100644 --- a/p2p/src/handshake.rs +++ b/p2p/src/handshake.rs @@ -51,19 +51,15 @@ impl Handshake { // send the first part of the handshake let sender_addr = conn.local_addr().unwrap(); let receiver_addr = conn.peer_addr().unwrap(); - let opt_err = serialize(&mut conn, - &Hand { - version: PROTOCOL_VERSION, - capabilities: FULL_SYNC, - nonce: nonce, - sender_addr: SockAddr(sender_addr), - receiver_addr: SockAddr(receiver_addr), - user_agent: USER_AGENT.to_string(), - }); - match opt_err { - Some(err) => return Err(err), - None => {} - } + try!(serialize(&mut conn, + &Hand { + version: PROTOCOL_VERSION, + capabilities: FULL_SYNC, + nonce: nonce, + sender_addr: SockAddr(sender_addr), + receiver_addr: SockAddr(receiver_addr), + user_agent: USER_AGENT.to_string(), + })); // deserialize the handshake response and do version negotiation let shake = try!(deserialize::(&mut conn)); @@ -127,16 +123,12 @@ impl Handshake { }; // send our reply with our info - let opt_err = serialize(&mut conn, - &Shake { - version: PROTOCOL_VERSION, - capabilities: FULL_SYNC, - user_agent: USER_AGENT.to_string(), - }); - match opt_err { - Some(err) => return Err(err), - None => {} - } + try!(serialize(&mut conn, + &Shake { + version: PROTOCOL_VERSION, + capabilities: FULL_SYNC, + user_agent: USER_AGENT.to_string(), + })); info!("Received connection from peer {:?}", peer_info); // when more than one protocol version is supported, choosing should go here diff --git a/p2p/src/msg.rs b/p2p/src/msg.rs index f247eddcc..8921d7c0b 100644 --- a/p2p/src/msg.rs +++ b/p2p/src/msg.rs @@ -18,7 +18,7 @@ use std::net::*; use num::FromPrimitive; -use core::ser::{self, Writeable, Readable, Writer, Reader, Error}; +use core::ser::{self, Writeable, Readable, Writer, Reader}; use types::*; @@ -72,12 +72,12 @@ impl MsgHeader { } impl Writeable for MsgHeader { - fn write(&self, writer: &mut Writer) -> Option { + fn write(&self, writer: &mut Writer) -> Result<(), ser::Error> { ser_multiwrite!(writer, [write_u8, self.magic[0]], [write_u8, self.magic[1]], [write_u8, self.msg_type as u8]); - None + Ok(()) } } @@ -111,14 +111,14 @@ pub struct Hand { } impl Writeable for Hand { - fn write(&self, writer: &mut Writer) -> Option { + fn write(&self, writer: &mut Writer) -> Result<(), ser::Error> { ser_multiwrite!(writer, [write_u32, self.version], [write_u32, self.capabilities.bits()], [write_u64, self.nonce]); self.sender_addr.write(writer); self.receiver_addr.write(writer); - writer.write_vec(&mut self.user_agent.clone().into_bytes()) + writer.write_bytes(self.user_agent.as_bytes()) } } @@ -153,12 +153,12 @@ pub struct Shake { } impl Writeable for Shake { - fn write(&self, writer: &mut Writer) -> Option { + fn write(&self, writer: &mut Writer) -> Result<(), ser::Error> { ser_multiwrite!(writer, [write_u32, self.version], [write_u32, self.capabilities.bits()], - [write_vec, &mut self.user_agent.as_bytes().to_vec()]); - None + [write_bytes, self.user_agent.as_bytes()]); + Ok(()) } } @@ -233,11 +233,11 @@ pub struct PeerError { } impl Writeable for PeerError { - fn write(&self, writer: &mut Writer) -> Option { + fn write(&self, writer: &mut Writer) -> Result<(), ser::Error> { ser_multiwrite!(writer, [write_u32, self.code], - [write_vec, &mut self.message.clone().into_bytes()]); - None + [write_bytes, self.message.as_bytes()]); + Ok(()) } } @@ -258,7 +258,7 @@ impl Readable for PeerError { pub struct SockAddr(pub SocketAddr); impl Writeable for SockAddr { - fn write(&self, writer: &mut Writer) -> Option { + fn write(&self, writer: &mut Writer) -> Result<(), ser::Error> { match self.0 { SocketAddr::V4(sav4) => { ser_multiwrite!(writer, @@ -267,14 +267,14 @@ impl Writeable for SockAddr { [write_u16, sav4.port()]); } SocketAddr::V6(sav6) => { - try_o!(writer.write_u8(1)); + try!(writer.write_u8(1)); for seg in &sav6.ip().segments() { - try_o!(writer.write_u16(*seg)); + try!(writer.write_u16(*seg)); } - try_o!(writer.write_u16(sav6.port())); + try!(writer.write_u16(sav6.port())); } } - None + Ok(()) } } From 245f0a8b565f3f40c79c550d9cc931506efb9f34 Mon Sep 17 00:00:00 2001 From: Merope Riddle Date: Tue, 1 Nov 2016 13:14:26 +0000 Subject: [PATCH 4/6] core/ser: add default implementations for most methods of `Writer` --- core/src/core/transaction.rs | 2 +- core/src/ser.rs | 89 +++++++++++++++++++----------------- 2 files changed, 49 insertions(+), 42 deletions(-) diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index 5af9ccd79..6884cc858 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -304,7 +304,7 @@ pub enum Output { impl Writeable for Output { fn write(&self, writer: &mut Writer) -> Result<(), ser::Error> { try!(writer.write_fixed_bytes(&self.commitment().unwrap())); - writer.write_bytes(&mut self.proof().unwrap().bytes()) + writer.write_bytes(&self.proof().unwrap().bytes()) } } diff --git a/core/src/ser.rs b/core/src/ser.rs index a0e09c7a3..4ffd516ed 100644 --- a/core/src/ser.rs +++ b/core/src/ser.rs @@ -21,7 +21,7 @@ use std::{error, fmt}; use std::io::{self, Write, Read}; -use byteorder::{ReadBytesExt, WriteBytesExt, BigEndian}; +use byteorder::{ByteOrder, ReadBytesExt, BigEndian}; /// Possible errors deriving from serializing or deserializing. #[derive(Debug)] @@ -85,21 +85,48 @@ pub trait AsFixedBytes { /// written to an underlying stream or container (depending on implementation). pub trait Writer { /// Writes a u8 as bytes - fn write_u8(&mut self, n: u8) -> Result<(), Error>; + fn write_u8(&mut self, n: u8) -> Result<(), Error> { + self.write_fixed_bytes(&[n]) + } + /// Writes a u16 as bytes - fn write_u16(&mut self, n: u16) -> Result<(), Error>; + fn write_u16(&mut self, n: u16) -> Result<(), Error> { + let mut bytes = [0; 2]; + BigEndian::write_u16(&mut bytes, n); + self.write_fixed_bytes(&bytes) + } + /// Writes a u32 as bytes - fn write_u32(&mut self, n: u32) -> Result<(), Error>; + fn write_u32(&mut self, n: u32) -> Result<(), Error> { + let mut bytes = [0; 4]; + BigEndian::write_u32(&mut bytes, n); + self.write_fixed_bytes(&bytes) + } + /// Writes a u64 as bytes - fn write_u64(&mut self, n: u64) -> Result<(), Error>; + fn write_u64(&mut self, n: u64) -> Result<(), Error> { + let mut bytes = [0; 8]; + BigEndian::write_u64(&mut bytes, n); + self.write_fixed_bytes(&bytes) + } + /// Writes a i64 as bytes - fn write_i64(&mut self, n: i64) -> Result<(), Error>; - /// Writes a variable length `Vec`, the length of the `Vec` is encoded as a + fn write_i64(&mut self, n: i64) -> Result<(), Error> { + let mut bytes = [0; 8]; + BigEndian::write_i64(&mut bytes, n); + self.write_fixed_bytes(&bytes) + } + + /// Writes a variable number of bytes. The length is encoded as a 64-bit /// prefix. - fn write_bytes(&mut self, data: &[u8]) -> Result<(), Error>; + fn write_bytes(&mut self, bytes: &AsFixedBytes) -> Result<(), Error> { + try!(self.write_u64(bytes.as_fixed_bytes().len() as u64)); + self.write_fixed_bytes(bytes) + } + /// Writes a fixed number of bytes from something that can turn itself into /// a `&[u8]`. The reader is expected to know the actual length on read. - fn write_fixed_bytes(&mut self, b32: &AsFixedBytes) -> Result<(), Error>; + fn write_fixed_bytes(&mut self, fixed: &AsFixedBytes) -> Result<(), Error>; } /// Implementations defined how different numbers and binary structures are @@ -225,38 +252,8 @@ struct BinWriter<'a> { } impl<'a> Writer for BinWriter<'a> { - fn write_u8(&mut self, n: u8) -> Result<(), Error> { - try!(self.sink.write_u8(n)); - Ok(()) - } - fn write_u16(&mut self, n: u16) -> Result<(), Error> { - try!(self.sink.write_u16::(n)); - Ok(()) - } - fn write_u32(&mut self, n: u32) -> Result<(), Error> { - try!(self.sink.write_u32::(n)); - Ok(()) - } - - fn write_u64(&mut self, n: u64) -> Result<(), Error> { - try!(self.sink.write_u64::(n)); - Ok(()) - } - - fn write_i64(&mut self, n: i64) -> Result<(), Error> { - try!(self.sink.write_i64::(n)); - Ok(()) - } - - - fn write_bytes(&mut self, data: &[u8]) -> Result<(), Error> { - try!(self.write_u64(data.len() as u64)); - try!(self.sink.write_all(data)); - Ok(()) - } - - fn write_fixed_bytes(&mut self, b32: &AsFixedBytes) -> Result<(), Error> { - let bs = b32.as_fixed_bytes(); + fn write_fixed_bytes(&mut self, fixed: &AsFixedBytes) -> Result<(), Error> { + let bs = fixed.as_fixed_bytes(); try!(self.sink.write_all(bs)); Ok(()) } @@ -276,6 +273,16 @@ impl_slice_bytes!(::secp::key::SecretKey); impl_slice_bytes!(::secp::Signature); impl_slice_bytes!(::secp::pedersen::Commitment); impl_slice_bytes!(Vec); +impl_slice_bytes!([u8; 1]); +impl_slice_bytes!([u8; 2]); +impl_slice_bytes!([u8; 4]); +impl_slice_bytes!([u8; 8]); + +impl<'a> AsFixedBytes for &'a [u8] { + fn as_fixed_bytes(&self) -> &[u8] { + *self + } +} impl AsFixedBytes for ::core::hash::Hash { fn as_fixed_bytes(&self) -> &[u8] { From ca89dae7e1206fbb097a563f486818dd513ddf95 Mon Sep 17 00:00:00 2001 From: Merope Riddle Date: Tue, 1 Nov 2016 01:37:06 +0000 Subject: [PATCH 5/6] core: unify Hashed and Writeable to simplify things and remove allocations --- core/src/core/block.rs | 9 +---- core/src/core/hash.rs | 69 ++++++++++++++++++++++++++---------- core/src/core/mod.rs | 12 +++---- core/src/core/transaction.rs | 40 +++++++-------------- core/src/pow/mod.rs | 7 ---- core/src/ser.rs | 19 ++++++++++ 6 files changed, 90 insertions(+), 66 deletions(-) diff --git a/core/src/core/block.rs b/core/src/core/block.rs index 5d3ee716e..3985705e0 100644 --- a/core/src/core/block.rs +++ b/core/src/core/block.rs @@ -26,7 +26,7 @@ use core::transaction::merkle_inputs_outputs; use core::{PROOFSIZE, REWARD}; use core::hash::{Hash, Hashed, ZERO_HASH}; use core::transaction::MAX_IN_OUT_LEN; -use ser::{self, Readable, Reader, Writeable, Writer, ser_vec}; +use ser::{self, Readable, Reader, Writeable, Writer}; /// Block header, fairly standard compared to other blockchains. pub struct BlockHeader { @@ -79,13 +79,6 @@ impl Writeable for BlockHeader { } } -impl Hashed for BlockHeader { - fn bytes(&self) -> Vec { - // no serialization errors are applicable in this specific case - ser_vec(self).unwrap() - } -} - /// A block as expressed in the MimbleWimble protocol. The reward is /// non-explicit, assumed to be deductible from block height (similar to /// bitcoin's schedule) and expressed as a global transaction fee (added v.H), diff --git a/core/src/core/hash.rs b/core/src/core/hash.rs index ad3328ec6..543b6ddfa 100644 --- a/core/src/core/hash.rs +++ b/core/src/core/hash.rs @@ -17,10 +17,12 @@ //! Primary hash function used in the protocol //! +use byteorder::{ByteOrder, WriteBytesExt, BigEndian}; use std::fmt; - use tiny_keccak::Keccak; +use ser::{self, AsFixedBytes}; + /// A hash to uniquely (or close enough) identify one of the main blockchain /// constructs. Used pervasively for blocks, transactions and ouputs. #[derive(Debug, Copy, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] @@ -56,27 +58,58 @@ impl Hash { pub const ZERO_HASH: Hash = Hash([0; 32]); -/// A trait for types that get their hash (double SHA256) from their byte -/// serialzation. -pub trait Hashed { - fn hash(&self) -> Hash { - let data = self.bytes(); - Hash(sha3(data)) - } - - fn bytes(&self) -> Vec; +/// Serializer that outputs a hash of the serialized object +pub struct HashWriter { + state: Keccak } -fn sha3(data: Vec) -> [u8; 32] { - let mut sha3 = Keccak::new_sha3_256(); - let mut buf = [0; 32]; - sha3.update(&data); - sha3.finalize(&mut buf); - buf +impl HashWriter { + fn finalize(self, output: &mut [u8]) { + self.state.finalize(output); + } +} + +impl Default for HashWriter { + fn default() -> HashWriter { + HashWriter { + state: Keccak::new_sha3_256() + } + } +} + +impl ser::Writer for HashWriter { + fn serialization_mode(&self) -> ser::SerializationMode { + ser::SerializationMode::Hash + } + + fn write_fixed_bytes(&mut self, b32: &AsFixedBytes) -> Result<(), ser::Error> { + self.state.update(b32.as_fixed_bytes()); + Ok(()) + } +} + +/// A trait for types that have a canonical hash +pub trait Hashed { + fn hash(&self) -> Hash; +} + +impl Hashed for W { + fn hash(&self) -> Hash { + let mut hasher = HashWriter::default(); + ser::Writeable::write(self, &mut hasher).unwrap(); + let mut ret = [0; 32]; + hasher.finalize(&mut ret); + Hash(ret) + } } impl Hashed for [u8] { - fn bytes(&self) -> Vec { - self.to_owned() + fn hash(&self) -> Hash { + let mut hasher = HashWriter::default(); + let mut ret = [0; 32]; + ser::Writer::write_bytes(&mut hasher, &self).unwrap(); + hasher.finalize(&mut ret); + Hash(ret) } } + diff --git a/core/src/core/mod.rs b/core/src/core/mod.rs index 1095c70aa..d1472eed7 100644 --- a/core/src/core/mod.rs +++ b/core/src/core/mod.rs @@ -154,12 +154,12 @@ impl Proof { /// Two hashes that will get hashed together in a Merkle tree to build the next /// level up. struct HPair(Hash, Hash); -impl Hashed for HPair { - fn bytes(&self) -> Vec { - let mut data = Vec::with_capacity(64); - data.extend_from_slice(self.0.to_slice()); - data.extend_from_slice(self.1.to_slice()); - return data; + +impl Writeable for HPair { + fn write(&self, writer: &mut Writer) -> Result<(), Error> { + try!(writer.write_bytes(&self.0.to_slice())); + try!(writer.write_bytes(&self.1.to_slice())); + Ok(()) } } /// An iterator over hashes in a vector that pairs them to build a row in a diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index 6884cc858..d1d6bcc02 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -16,7 +16,7 @@ use core::Committed; use core::MerkleRow; -use core::hash::{Hashed, Hash}; +use core::hash::{Hash, Hashed}; use ser::{self, Reader, Writer, Readable, Writeable}; use secp::{self, Secp256k1, Message, Signature}; @@ -283,13 +283,6 @@ impl Input { } } -/// The hash of an input is the hash of the output hash it references. -impl Hashed for Input { - fn bytes(&self) -> Vec { - self.output_hash().to_vec() - } -} - #[derive(Debug, Copy, Clone)] pub enum Output { BlindOutput { @@ -303,8 +296,12 @@ pub enum Output { /// an Output as binary. impl Writeable for Output { fn write(&self, writer: &mut Writer) -> Result<(), ser::Error> { + // The hash of an output is only the hash of its commitment. try!(writer.write_fixed_bytes(&self.commitment().unwrap())); - writer.write_bytes(&self.proof().unwrap().bytes()) + if writer.serialization_mode() == ser::SerializationMode::Full { + try!(writer.write_bytes(&self.proof().unwrap().bytes())) + } + Ok(()) } } @@ -363,17 +360,6 @@ impl Output { } } -/// The hash of an output is the hash of its commitment. -impl Hashed for Output { - fn bytes(&self) -> Vec { - if let &Output::BlindOutput { commit, .. } = self { - return commit.bytes().to_vec(); - } else { - panic!("cannot hash an overt output"); - } - } -} - /// Utility function to calculate the Merkle root of vectors of inputs and /// outputs. pub fn merkle_inputs_outputs(inputs: &Vec, outputs: &Vec) -> Hash { @@ -418,12 +404,12 @@ mod test { let ref secp = new_secp(); let tx = tx2i1o(secp, &mut rng); - let mut btx = tx.blind(&secp).unwrap(); + let btx = tx.blind(&secp).unwrap(); let mut vec = Vec::new(); serialize(&mut vec, &btx).expect("serialization failed"); // let mut dtx = Transaction::read(&mut BinReader { source: &mut &vec[..] // }).unwrap(); - let mut dtx: Transaction = deserialize(&mut &vec[..]).unwrap(); + let dtx: Transaction = deserialize(&mut &vec[..]).unwrap(); assert_eq!(dtx.fee, 1); assert_eq!(dtx.inputs.len(), 2); assert_eq!(dtx.outputs.len(), 1); @@ -437,15 +423,15 @@ mod test { let ref secp = new_secp(); let tx = tx2i1o(secp, &mut rng); - let mut btx = tx.blind(&secp).unwrap(); + let btx = tx.blind(&secp).unwrap(); let mut vec = Vec::new(); assert!(serialize(&mut vec, &btx).is_ok()); - let mut dtx: Transaction = deserialize(&mut &vec[..]).unwrap(); + let dtx: Transaction = deserialize(&mut &vec[..]).unwrap(); let mut vec2 = Vec::new(); assert!(serialize(&mut vec2, &btx).is_ok()); - let mut dtx2: Transaction = deserialize(&mut &vec2[..]).unwrap(); + let dtx2: Transaction = deserialize(&mut &vec2[..]).unwrap(); assert_eq!(btx.hash(), dtx.hash()); assert_eq!(dtx.hash(), dtx2.hash()); @@ -532,10 +518,10 @@ mod test { let mut rng = OsRng::new().unwrap(); let tx1 = tx2i1o(secp, &mut rng); - let mut btx1 = tx1.blind(&secp).unwrap(); + let btx1 = tx1.blind(&secp).unwrap(); let tx2 = tx1i1o(secp, &mut rng); - let mut btx2 = tx2.blind(&secp).unwrap(); + let btx2 = tx2.blind(&secp).unwrap(); if btx1.hash() == btx2.hash() { panic!("diff txs have same hash") diff --git a/core/src/pow/mod.rs b/core/src/pow/mod.rs index d17da511b..7047c10c6 100644 --- a/core/src/pow/mod.rs +++ b/core/src/pow/mod.rs @@ -85,13 +85,6 @@ impl Writeable for PowHeader { } } -impl Hashed for PowHeader { - fn bytes(&self) -> Vec { - // no serialization errors are applicable in this specific case - ser_vec(self).unwrap() - } -} - impl PowHeader { fn from_block(b: &Block) -> PowHeader { let ref h = b.header; diff --git a/core/src/ser.rs b/core/src/ser.rs index 4ffd516ed..54ed99255 100644 --- a/core/src/ser.rs +++ b/core/src/ser.rs @@ -81,9 +81,24 @@ pub trait AsFixedBytes { fn as_fixed_bytes(&self) -> &[u8]; } + +/// Signal to a serializable object how much of its data should be serialized +#[derive(Copy, Clone, PartialEq, Eq)] +pub enum SerializationMode { + /// Serialize everything sufficiently to fully reconstruct the object + Full, + /// Serialize the data that defines the object + Hash, + /// Serialize everything that a signer of the object should know + SigHash +} + /// Implementations defined how different numbers and binary structures are /// written to an underlying stream or container (depending on implementation). pub trait Writer { + /// The mode this serializer is writing in + fn serialization_mode(&self) -> SerializationMode; + /// Writes a u8 as bytes fn write_u8(&mut self, n: u8) -> Result<(), Error> { self.write_fixed_bytes(&[n]) @@ -252,6 +267,10 @@ struct BinWriter<'a> { } impl<'a> Writer for BinWriter<'a> { + fn serialization_mode(&self) -> SerializationMode { + SerializationMode::Full + } + fn write_fixed_bytes(&mut self, fixed: &AsFixedBytes) -> Result<(), Error> { let bs = fixed.as_fixed_bytes(); try!(self.sink.write_all(bs)); From 33ccfe3b65ef5dc896fd9b8d2167300af7a9d749 Mon Sep 17 00:00:00 2001 From: Merope Riddle Date: Tue, 1 Nov 2016 01:44:11 +0000 Subject: [PATCH 6/6] core: remove several unused imports --- core/src/core/hash.rs | 2 +- core/src/core/mod.rs | 13 +++---------- core/src/core/transaction.rs | 1 - core/src/pow/cuckoo.rs | 1 - core/src/pow/mod.rs | 8 +++----- 5 files changed, 7 insertions(+), 18 deletions(-) diff --git a/core/src/core/hash.rs b/core/src/core/hash.rs index 543b6ddfa..770a45ae0 100644 --- a/core/src/core/hash.rs +++ b/core/src/core/hash.rs @@ -17,7 +17,7 @@ //! Primary hash function used in the protocol //! -use byteorder::{ByteOrder, WriteBytesExt, BigEndian}; +use byteorder::{ByteOrder, BigEndian}; use std::fmt; use tiny_keccak::Keccak; diff --git a/core/src/core/mod.rs b/core/src/core/mod.rs index d1472eed7..af5f7368c 100644 --- a/core/src/core/mod.rs +++ b/core/src/core/mod.rs @@ -18,25 +18,18 @@ pub mod block; pub mod hash; pub mod transaction; #[allow(dead_code)] -#[macro_use] pub use self::block::{Block, BlockHeader}; pub use self::transaction::{Transaction, Input, Output, TxProof}; use self::hash::{Hash, Hashed, ZERO_HASH}; -use ser::{Writeable, Writer, Error, ser_vec}; - -use time; +use ser::{Writeable, Writer, Error}; use std::fmt; use std::cmp::Ordering; -use secp; -use secp::{Secp256k1, Signature, Message}; -use secp::key::SecretKey; +use secp::{self, Secp256k1}; use secp::pedersen::*; -use tiny_keccak::Keccak; - /// The block subsidy amount pub const REWARD: u64 = 1_000_000_000; @@ -198,7 +191,7 @@ impl MerkleRow { #[cfg(test)] mod test { use super::*; - use super::hash::{Hash, Hashed, ZERO_HASH}; + use core::hash::ZERO_HASH; use secp; use secp::Secp256k1; use secp::key::SecretKey; diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index d1d6bcc02..33c715dbd 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -378,7 +378,6 @@ mod test { use secp::{self, Secp256k1}; use secp::key::SecretKey; - use rand::Rng; use rand::os::OsRng; fn new_secp() -> Secp256k1 { diff --git a/core/src/pow/cuckoo.rs b/core/src/pow/cuckoo.rs index 6d0a4615c..544aef68b 100644 --- a/core/src/pow/cuckoo.rs +++ b/core/src/pow/cuckoo.rs @@ -19,7 +19,6 @@ use std::collections::HashSet; use std::cmp; -use std::fmt; use crypto::digest::Digest; use crypto::sha2::Sha256; diff --git a/core/src/pow/mod.rs b/core/src/pow/mod.rs index 7047c10c6..d2069721f 100644 --- a/core/src/pow/mod.rs +++ b/core/src/pow/mod.rs @@ -27,12 +27,12 @@ mod cuckoo; use time; -use core::{Block, BlockHeader, Proof, PROOFSIZE}; +use core::{Block, Proof, PROOFSIZE}; use core::hash::{Hash, Hashed}; use pow::cuckoo::{Cuckoo, Miner, Error}; use ser; -use ser::{Writeable, Writer, ser_vec}; +use ser::{Writeable, Writer}; /// Default Cuckoo Cycle size shift used is 28. We may decide to increase it. /// when difficuty increases. @@ -169,9 +169,7 @@ fn pow_size(b: &Block, target: Proof, sizeshift: u32) -> Result<(Proof, u64), Er #[cfg(test)] mod test { use super::*; - use core::{BlockHeader, Proof}; - use core::hash::Hash; - use std::time::Instant; + use core::Proof; use genesis; #[test]