From 15488b3e40d6f093d4977c36292cf0f5071c854f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 22 Sep 2016 14:50:00 +0200 Subject: [PATCH] Fixing output of eth_call and Bytes deserialization (#2230) * Fixing eth_call to builtins * Fixing bytes deserialization * Removing comment --- ethcore/src/builtin.rs | 106 +++++++++++++++---------------------- ethcore/src/engines/mod.rs | 4 +- ethcore/src/executive.rs | 1 - rpc/src/v1/types/bytes.rs | 18 ++++++- util/src/bytes.rs | 86 +++++++++++++++++++++++++++++- 5 files changed, 147 insertions(+), 68 deletions(-) diff --git a/ethcore/src/builtin.rs b/ethcore/src/builtin.rs index 2825a2a12..e8eb0ed68 100644 --- a/ethcore/src/builtin.rs +++ b/ethcore/src/builtin.rs @@ -17,14 +17,15 @@ use crypto::sha2::Sha256 as Sha256Digest; use crypto::ripemd160::Ripemd160 as Ripemd160Digest; use crypto::digest::Digest; -use util::*; +use std::cmp::min; +use util::{U256, H256, Hashable, FixedHash, BytesRef}; use ethkey::{Signature, recover as ec_recover}; use ethjson; /// Native implementation of a built-in contract. pub trait Impl: Send + Sync { /// execute this built-in on the given input, writing to the given output. - fn execute(&self, input: &[u8], out: &mut [u8]); + fn execute(&self, input: &[u8], output: &mut BytesRef); } /// A gas pricing scheme for built-in contracts. @@ -56,7 +57,7 @@ impl Builtin { pub fn cost(&self, s: usize) -> U256 { self.pricer.cost(s) } /// Simple forwarder for execute. - pub fn execute(&self, input: &[u8], output: &mut[u8]) { self.native.execute(input, output) } + pub fn execute(&self, input: &[u8], output: &mut BytesRef) { self.native.execute(input, output) } } impl From for Builtin { @@ -108,14 +109,13 @@ struct Sha256; struct Ripemd160; impl Impl for Identity { - fn execute(&self, input: &[u8], output: &mut [u8]) { - let len = min(input.len(), output.len()); - output[..len].copy_from_slice(&input[..len]); + fn execute(&self, input: &[u8], output: &mut BytesRef) { + output.write(0, input); } } impl Impl for EcRecover { - fn execute(&self, i: &[u8], output: &mut [u8]) { + fn execute(&self, i: &[u8], output: &mut BytesRef) { let len = min(i.len(), 128); let mut input = [0; 128]; @@ -135,58 +135,34 @@ impl Impl for EcRecover { if s.is_valid() { if let Ok(p) = ec_recover(&s, &hash) { let r = p.sha3(); - - let out_len = min(output.len(), 32); - - for x in &mut output[0.. min(12, out_len)] { - *x = 0; - } - - if out_len > 12 { - output[12..out_len].copy_from_slice(&r[12..out_len]); - } + output.write(0, &[0; 12]); + output.write(12, &r[12..r.len()]); } } } } impl Impl for Sha256 { - fn execute(&self, input: &[u8], output: &mut [u8]) { - let out_len = min(output.len(), 32); - + fn execute(&self, input: &[u8], output: &mut BytesRef) { let mut sha = Sha256Digest::new(); sha.input(input); - if out_len == 32 { - sha.result(&mut output[0..32]); - } else { - let mut out = [0; 32]; - sha.result(&mut out); + let mut out = [0; 32]; + sha.result(&mut out); - output.copy_from_slice(&out[..out_len]) - } + output.write(0, &out); } } impl Impl for Ripemd160 { - fn execute(&self, input: &[u8], output: &mut [u8]) { - let out_len = min(output.len(), 32); - + fn execute(&self, input: &[u8], output: &mut BytesRef) { let mut sha = Ripemd160Digest::new(); sha.input(input); - for x in &mut output[0.. min(12, out_len)] { - *x = 0; - } + let mut out = [0; 32]; + sha.result(&mut out[12..32]); - if out_len >= 32 { - sha.result(&mut output[12..32]); - } else if out_len > 12 { - let mut out = [0; 20]; - sha.result(&mut out); - - output.copy_from_slice(&out[12..out_len]) - } + output.write(0, &out); } } @@ -194,7 +170,7 @@ impl Impl for Ripemd160 { mod tests { use super::{Builtin, Linear, ethereum_builtin, Pricer}; use ethjson; - use util::U256; + use util::{U256, BytesRef}; #[test] fn identity() { @@ -203,15 +179,15 @@ mod tests { let i = [0u8, 1, 2, 3]; let mut o2 = [255u8; 2]; - f.execute(&i[..], &mut o2[..]); + f.execute(&i[..], &mut BytesRef::Fixed(&mut o2[..])); assert_eq!(i[0..2], o2); let mut o4 = [255u8; 4]; - f.execute(&i[..], &mut o4[..]); + f.execute(&i[..], &mut BytesRef::Fixed(&mut o4[..])); assert_eq!(i, o4); let mut o8 = [255u8; 8]; - f.execute(&i[..], &mut o8[..]); + f.execute(&i[..], &mut BytesRef::Fixed(&mut o8[..])); assert_eq!(i, o8[..4]); assert_eq!([255u8; 4], o8[4..]); } @@ -224,16 +200,20 @@ mod tests { let i = [0u8; 0]; let mut o = [255u8; 32]; - f.execute(&i[..], &mut o[..]); + f.execute(&i[..], &mut BytesRef::Fixed(&mut o[..])); assert_eq!(&o[..], &(FromHex::from_hex("e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855").unwrap())[..]); let mut o8 = [255u8; 8]; - f.execute(&i[..], &mut o8[..]); + f.execute(&i[..], &mut BytesRef::Fixed(&mut o8[..])); assert_eq!(&o8[..], &(FromHex::from_hex("e3b0c44298fc1c14").unwrap())[..]); let mut o34 = [255u8; 34]; - f.execute(&i[..], &mut o34[..]); + f.execute(&i[..], &mut BytesRef::Fixed(&mut o34[..])); assert_eq!(&o34[..], &(FromHex::from_hex("e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855ffff").unwrap())[..]); + + let mut ov = vec![]; + f.execute(&i[..], &mut BytesRef::Flexible(&mut ov)); + assert_eq!(&ov[..], &(FromHex::from_hex("e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855").unwrap())[..]); } #[test] @@ -244,15 +224,15 @@ mod tests { let i = [0u8; 0]; let mut o = [255u8; 32]; - f.execute(&i[..], &mut o[..]); + f.execute(&i[..], &mut BytesRef::Fixed(&mut o[..])); assert_eq!(&o[..], &(FromHex::from_hex("0000000000000000000000009c1185a5c5e9fc54612808977ee8f548b2258d31").unwrap())[..]); let mut o8 = [255u8; 8]; - f.execute(&i[..], &mut o8[..]); + f.execute(&i[..], &mut BytesRef::Fixed(&mut o8[..])); assert_eq!(&o8[..], &(FromHex::from_hex("0000000000000000").unwrap())[..]); let mut o34 = [255u8; 34]; - f.execute(&i[..], &mut o34[..]); + f.execute(&i[..], &mut BytesRef::Fixed(&mut o34[..])); assert_eq!(&o34[..], &(FromHex::from_hex("0000000000000000000000009c1185a5c5e9fc54612808977ee8f548b2258d31ffff").unwrap())[..]); } @@ -272,46 +252,46 @@ mod tests { let i = FromHex::from_hex("47173285a8d7341e5e972fc677286384f802f8ef42a5ec5f03bbfa254cb01fad000000000000000000000000000000000000000000000000000000000000001b650acf9d3f5f0a2c799776a1254355d5f4061762a237396a99a0e0e3fc2bcd6729514a0dacb2e623ac4abd157cb18163ff942280db4d5caad66ddf941ba12e03").unwrap(); let mut o = [255u8; 32]; - f.execute(&i[..], &mut o[..]); + f.execute(&i[..], &mut BytesRef::Fixed(&mut o[..])); assert_eq!(&o[..], &(FromHex::from_hex("000000000000000000000000c08b5542d177ac6686946920409741463a15dddb").unwrap())[..]); let mut o8 = [255u8; 8]; - f.execute(&i[..], &mut o8[..]); + f.execute(&i[..], &mut BytesRef::Fixed(&mut o8[..])); assert_eq!(&o8[..], &(FromHex::from_hex("0000000000000000").unwrap())[..]); let mut o34 = [255u8; 34]; - f.execute(&i[..], &mut o34[..]); + f.execute(&i[..], &mut BytesRef::Fixed(&mut o34[..])); assert_eq!(&o34[..], &(FromHex::from_hex("000000000000000000000000c08b5542d177ac6686946920409741463a15dddbffff").unwrap())[..]); let i_bad = FromHex::from_hex("47173285a8d7341e5e972fc677286384f802f8ef42a5ec5f03bbfa254cb01fad000000000000000000000000000000000000000000000000000000000000001a650acf9d3f5f0a2c799776a1254355d5f4061762a237396a99a0e0e3fc2bcd6729514a0dacb2e623ac4abd157cb18163ff942280db4d5caad66ddf941ba12e03").unwrap(); let mut o = [255u8; 32]; - f.execute(&i_bad[..], &mut o[..]); + f.execute(&i_bad[..], &mut BytesRef::Fixed(&mut o[..])); assert_eq!(&o[..], &(FromHex::from_hex("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap())[..]); let i_bad = FromHex::from_hex("47173285a8d7341e5e972fc677286384f802f8ef42a5ec5f03bbfa254cb01fad000000000000000000000000000000000000000000000000000000000000001b000000000000000000000000000000000000000000000000000000000000001b0000000000000000000000000000000000000000000000000000000000000000").unwrap(); let mut o = [255u8; 32]; - f.execute(&i_bad[..], &mut o[..]); + f.execute(&i_bad[..], &mut BytesRef::Fixed(&mut o[..])); assert_eq!(&o[..], &(FromHex::from_hex("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap())[..]); let i_bad = FromHex::from_hex("47173285a8d7341e5e972fc677286384f802f8ef42a5ec5f03bbfa254cb01fad000000000000000000000000000000000000000000000000000000000000001b0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001b").unwrap(); let mut o = [255u8; 32]; - f.execute(&i_bad[..], &mut o[..]); + f.execute(&i_bad[..], &mut BytesRef::Fixed(&mut o[..])); assert_eq!(&o[..], &(FromHex::from_hex("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap())[..]); let i_bad = FromHex::from_hex("47173285a8d7341e5e972fc677286384f802f8ef42a5ec5f03bbfa254cb01fad000000000000000000000000000000000000000000000000000000000000001bffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff000000000000000000000000000000000000000000000000000000000000001b").unwrap(); let mut o = [255u8; 32]; - f.execute(&i_bad[..], &mut o[..]); + f.execute(&i_bad[..], &mut BytesRef::Fixed(&mut o[..])); assert_eq!(&o[..], &(FromHex::from_hex("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap())[..]); let i_bad = FromHex::from_hex("47173285a8d7341e5e972fc677286384f802f8ef42a5ec5f03bbfa254cb01fad000000000000000000000000000000000000000000000000000000000000001b000000000000000000000000000000000000000000000000000000000000001bffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap(); let mut o = [255u8; 32]; - f.execute(&i_bad[..], &mut o[..]); + f.execute(&i_bad[..], &mut BytesRef::Fixed(&mut o[..])); assert_eq!(&o[..], &(FromHex::from_hex("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap())[..]); // TODO: Should this (corrupted version of the above) fail rather than returning some address? /* let i_bad = FromHex::from_hex("48173285a8d7341e5e972fc677286384f802f8ef42a5ec5f03bbfa254cb01fad000000000000000000000000000000000000000000000000000000000000001b650acf9d3f5f0a2c799776a1254355d5f4061762a237396a99a0e0e3fc2bcd6729514a0dacb2e623ac4abd157cb18163ff942280db4d5caad66ddf941ba12e03").unwrap(); let mut o = [255u8; 32]; - f.execute(&i_bad[..], &mut o[..]); + f.execute(&i_bad[..], &mut BytesRef::Fixed(&mut o[..])); assert_eq!(&o[..], &(FromHex::from_hex("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap())[..]);*/ } @@ -336,7 +316,7 @@ mod tests { let i = [0u8, 1, 2, 3]; let mut o = [255u8; 4]; - b.execute(&i[..], &mut o[..]); + b.execute(&i[..], &mut BytesRef::Fixed(&mut o[..])); assert_eq!(i, o); } @@ -357,7 +337,7 @@ mod tests { let i = [0u8, 1, 2, 3]; let mut o = [255u8; 4]; - b.execute(&i[..], &mut o[..]); + b.execute(&i[..], &mut BytesRef::Fixed(&mut o[..])); assert_eq!(i, o); } -} \ No newline at end of file +} diff --git a/ethcore/src/engines/mod.rs b/ethcore/src/engines/mod.rs index 599721f86..5234553ef 100644 --- a/ethcore/src/engines/mod.rs +++ b/ethcore/src/engines/mod.rs @@ -45,7 +45,7 @@ pub trait Engine : Sync + Send { fn extra_info(&self, _header: &Header) -> HashMap { HashMap::new() } /// Additional information. - fn additional_params(&self) -> HashMap { HashMap::new() } + fn additional_params(&self) -> HashMap { HashMap::new() } /// Get the general parameters of the chain. fn params(&self) -> &CommonParams; @@ -126,7 +126,7 @@ pub trait Engine : Sync + Send { fn cost_of_builtin(&self, a: &Address, input: &[u8]) -> U256 { self.builtins().get(a).unwrap().cost(input.len()) } /// Execution the builtin contract `a` on `input` and return `output`. /// Panics if `is_builtin(a)` is not true. - fn execute_builtin(&self, a: &Address, input: &[u8], output: &mut [u8]) { self.builtins().get(a).unwrap().execute(input, output); } + fn execute_builtin(&self, a: &Address, input: &[u8], output: &mut BytesRef) { self.builtins().get(a).unwrap().execute(input, output); } // TODO: sealing stuff - though might want to leave this for later. } diff --git a/ethcore/src/executive.rs b/ethcore/src/executive.rs index 332eda190..8f8b534ee 100644 --- a/ethcore/src/executive.rs +++ b/ethcore/src/executive.rs @@ -193,7 +193,6 @@ impl<'a> Executive<'a> { data: Some(t.data.clone()), call_type: CallType::Call, }; - // TODO: move output upstream let mut out = vec![]; (self.call(params, &mut substate, BytesRef::Flexible(&mut out), &mut tracer, &mut vm_tracer), out) } diff --git a/rpc/src/v1/types/bytes.rs b/rpc/src/v1/types/bytes.rs index 09c899057..acf2c83ef 100644 --- a/rpc/src/v1/types/bytes.rs +++ b/rpc/src/v1/types/bytes.rs @@ -70,7 +70,7 @@ impl Visitor for BytesVisitor { type Value = Bytes; fn visit_str(&mut self, value: &str) -> Result where E: Error { - if value.len() >= 2 && &value[0..2] == "0x" { + if value.len() >= 2 && &value[0..2] == "0x" && value.len() & 1 == 0 { Ok(Bytes::new(FromHex::from_hex(&value[2..]).unwrap_or_else(|_| vec![]))) } else { Err(Error::custom("invalid hex")) @@ -95,5 +95,21 @@ mod tests { let serialized = serde_json::to_string(&bytes).unwrap(); assert_eq!(serialized, r#""0x0123456789abcdef""#); } + + #[test] + fn test_bytes_deserialize() { + let bytes1: Result = serde_json::from_str(r#""""#); + let bytes2: Result = serde_json::from_str(r#""0x123""#); + + let bytes3: Bytes = serde_json::from_str(r#""0x""#).unwrap(); + let bytes4: Bytes = serde_json::from_str(r#""0x12""#).unwrap(); + let bytes5: Bytes = serde_json::from_str(r#""0x0123""#).unwrap(); + + assert!(bytes1.is_err()); + assert!(bytes2.is_err()); + assert_eq!(bytes3, Bytes(vec![])); + assert_eq!(bytes4, Bytes(vec![0x12])); + assert_eq!(bytes5, Bytes(vec![0x1, 0x23])); + } } diff --git a/util/src/bytes.rs b/util/src/bytes.rs index 7c5e929f4..80b44c0e7 100644 --- a/util/src/bytes.rs +++ b/util/src/bytes.rs @@ -20,6 +20,7 @@ //! as use std::fmt; +use std::cmp::min; use std::ops::{Deref, DerefMut}; /// Slice pretty print helper @@ -71,6 +72,32 @@ pub enum BytesRef<'a> { Fixed(&'a mut [u8]) } +impl<'a> BytesRef<'a> { + /// Writes given `input` to this `BytesRef` starting at `offset`. + /// Returns number of bytes written to the ref. + /// NOTE can return number greater then `input.len()` in case flexible vector had to be extended. + pub fn write(&mut self, offset: usize, input: &[u8]) -> usize { + match *self { + BytesRef::Flexible(ref mut data) => { + let data_len = data.len(); + let wrote = input.len() + if data_len > offset { 0 } else { offset - data_len }; + + data.resize(offset, 0); + data.extend_from_slice(input); + wrote + }, + BytesRef::Fixed(ref mut data) if offset < data.len() => { + let max = min(data.len() - offset, input.len()); + for i in 0..max { + data[offset + i] = input[i]; + } + max + }, + _ => 0 + } + } +} + impl<'a> Deref for BytesRef<'a> { type Target = [u8]; @@ -92,4 +119,61 @@ impl <'a> DerefMut for BytesRef<'a> { } /// Vector of bytes. -pub type Bytes = Vec; \ No newline at end of file +pub type Bytes = Vec; + +#[cfg(test)] +mod tests { + use super::BytesRef; + + #[test] + fn should_write_bytes_to_fixed_bytesref() { + // given + let mut data1 = vec![0, 0, 0]; + let mut data2 = vec![0, 0, 0]; + let (res1, res2) = { + let mut bytes1 = BytesRef::Fixed(&mut data1[..]); + let mut bytes2 = BytesRef::Fixed(&mut data2[1..2]); + + // when + let res1 = bytes1.write(1, &[1, 1, 1]); + let res2 = bytes2.write(3, &[1, 1, 1]); + (res1, res2) + }; + + // then + assert_eq!(&data1, &[0, 1, 1]); + assert_eq!(res1, 2); + + assert_eq!(&data2, &[0, 0, 0]); + assert_eq!(res2, 0); + } + + #[test] + fn should_write_bytes_to_flexible_bytesref() { + // given + let mut data1 = vec![0, 0, 0]; + let mut data2 = vec![0, 0, 0]; + let mut data3 = vec![0, 0, 0]; + let (res1, res2, res3) = { + let mut bytes1 = BytesRef::Flexible(&mut data1); + let mut bytes2 = BytesRef::Flexible(&mut data2); + let mut bytes3 = BytesRef::Flexible(&mut data3); + + // when + let res1 = bytes1.write(1, &[1, 1, 1]); + let res2 = bytes2.write(3, &[1, 1, 1]); + let res3 = bytes3.write(5, &[1, 1, 1]); + (res1, res2, res3) + }; + + // then + assert_eq!(&data1, &[0, 1, 1, 1]); + assert_eq!(res1, 3); + + assert_eq!(&data2, &[0, 0, 0, 1, 1, 1]); + assert_eq!(res2, 3); + + assert_eq!(&data3, &[0, 0, 0, 0, 0, 1, 1, 1]); + assert_eq!(res3, 5); + } +}