From 51dfbd36707e1c255d4bcc4409700d78b8b7bfeb Mon Sep 17 00:00:00 2001 From: arkpar Date: Mon, 24 Sep 2018 17:28:13 +0200 Subject: [PATCH 1/3] Fixed extrinsic encoding --- .../src/generic/unchecked_extrinsic.rs | 32 ++++++++++++++----- .../src/generic/unchecked_mortal_extrinsic.rs | 27 ++++++++++------ node/executor/src/lib.rs | 2 +- 3 files changed, 43 insertions(+), 18 deletions(-) diff --git a/core/sr-primitives/src/generic/unchecked_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_extrinsic.rs index 4f3a32b34d156..3ce9aa3188dd9 100644 --- a/core/sr-primitives/src/generic/unchecked_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_extrinsic.rs @@ -103,9 +103,9 @@ where fn decode(input: &mut I) -> Option { // This is a little more complicated than usual since the binary format must be compatible // with substrate's generic `Vec` type. Basically this just means accepting that there - // will be a prefix of u32, which has the total number of bytes following (we don't need + // will be a prefix of vector length (we don't need // to use this). - let _length_do_not_remove_me_see_above: u32 = Decode::decode(input)?; + let _length_do_not_remove_me_see_above: Vec<()> = Decode::decode(input)?; Some(UncheckedExtrinsic { signature: Decode::decode(input)?, @@ -125,15 +125,14 @@ where fn encode(&self) -> Vec { let mut v = Vec::new(); - // need to prefix with the total length as u32 to ensure it's binary comptible with - // Vec. we'll make room for it here, then overwrite once we know the length. - v.extend(&[0u8; 4]); - self.signature.encode_to(&mut v); self.function.encode_to(&mut v); - let length = (v.len() - 4) as u32; - length.using_encoded(|s| v[0..4].copy_from_slice(s)); + // need to prefix with the total length to ensure it's binary comptible with + // Vec. + let mut length: Vec<()> = Vec::new(); + length.resize(v.len(), ()); + length.using_encoded(|s| { v.splice(0..0, s.iter().cloned()); }); v } @@ -150,3 +149,20 @@ impl fmt::Debug for UncheckedExtrinsic; + let ex = Extrinsic::new_unsigned(42); + let encoded = ex.encode(); + let decoded = Extrinsic::decode(&mut encoded.as_slice()).unwrap(); + assert_eq!(decoded, ex); + let as_vec: Vec = Decode::decode(&mut encoded.as_slice()).unwrap(); + assert_eq!(as_vec.encode(), encoded); + } +} diff --git a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs index c494bc00ff428..e45c778ff3e6b 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs @@ -113,9 +113,9 @@ where fn decode(input: &mut I) -> Option { // This is a little more complicated than usual since the binary format must be compatible // with substrate's generic `Vec` type. Basically this just means accepting that there - // will be a prefix of u32, which has the total number of bytes following (we don't need + // will be a prefix of vector length (we don't need // to use this). - let _length_do_not_remove_me_see_above: u32 = Decode::decode(input)?; + let _length_do_not_remove_me_see_above: Vec<()> = Decode::decode(input)?; let version = input.read_byte()?; @@ -143,10 +143,6 @@ where fn encode(&self) -> Vec { let mut v = Vec::new(); - // need to prefix with the total length as u32 to ensure it's binary comptible with - // Vec. we'll make room for it here, then overwrite once we know the length. - v.extend(&[0u8; 4]); - // 1 byte version id. match self.signature.as_ref() { Some(s) => { @@ -159,8 +155,11 @@ where } self.function.encode_to(&mut v); - let length = (v.len() - 4) as u32; - length.using_encoded(|s| v[0..4].copy_from_slice(s)); + // need to prefix with the total length to ensure it's binary comptible with + // Vec. + let mut length: Vec<()> = Vec::new(); + length.resize(v.len(), ()); + length.using_encoded(|s| { v.splice(0..0, s.iter().cloned()); }); v } @@ -275,4 +274,14 @@ mod tests { assert!(ux.is_signed()); assert_eq!(>::check(ux, &TestContext), Err("bad signature in extrinsic")); } -} \ No newline at end of file + + #[test] + fn encoding_matches_vec() { + let ex = Ex::new_unsigned(DUMMY_FUNCTION); + let encoded = ex.encode(); + let decoded = Ex::decode(&mut encoded.as_slice()).unwrap(); + assert_eq!(decoded, ex); + let as_vec: Vec = Decode::decode(&mut encoded.as_slice()).unwrap(); + assert_eq!(as_vec.encode(), encoded); + } +} diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index 4282d0b7c4aca..f24cb71e20145 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -312,7 +312,7 @@ mod tests { construct_block( 2, block1(false).1, - hex!("703efd5294ae5677c3c975593c64796cce27b471f27936c0cf31fda0a55ede19").into(), + hex!("022372996f312fc57b4649a8e3427a989803652d6d08735f4e4c8510f6b89ff8").into(), None, vec![ CheckedExtrinsic { From 8b4b38447046cd315643853aacf36d69f09a8adb Mon Sep 17 00:00:00 2001 From: arkpar Date: Tue, 25 Sep 2018 17:31:23 +0200 Subject: [PATCH 2/3] Reserve heuristic --- core/sr-primitives/src/generic/mod.rs | 24 ++++++++++++++ .../src/generic/unchecked_extrinsic.rs | 16 +++------- .../src/generic/unchecked_mortal_extrinsic.rs | 32 +++++++------------ 3 files changed, 40 insertions(+), 32 deletions(-) diff --git a/core/sr-primitives/src/generic/mod.rs b/core/sr-primitives/src/generic/mod.rs index 6732722c4db0a..85d89a38ace28 100644 --- a/core/sr-primitives/src/generic/mod.rs +++ b/core/sr-primitives/src/generic/mod.rs @@ -35,3 +35,27 @@ pub use self::checked_extrinsic::CheckedExtrinsic; pub use self::header::Header; pub use self::block::{Block, SignedBlock, BlockId}; pub use self::digest::{Digest, DigestItem, DigestItemRef}; + +use codec::Encode; + +fn encode_with_vec_prefix)>(encoder: F) -> Vec { + let size = ::std::mem::size_of::(); + let reserve = match size { + 0...0b00111111 => 1, + 0...0b00111111_11111111 => 2, + _ => 4, + }; + let mut v = Vec::with_capacity(reserve + size); + v.resize(reserve, 0); + encoder(&mut v); + + // need to prefix with the total length to ensure it's binary comptible with + // Vec. + let mut length: Vec<()> = Vec::new(); + length.resize(v.len() - reserve, ()); + length.using_encoded(|s| { + v.splice(0..reserve, s.iter().cloned()); + }); + + v +} diff --git a/core/sr-primitives/src/generic/unchecked_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_extrinsic.rs index 3ce9aa3188dd9..b5cd01c2cd9e3 100644 --- a/core/sr-primitives/src/generic/unchecked_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_extrinsic.rs @@ -123,18 +123,10 @@ where Call: Encode, { fn encode(&self) -> Vec { - let mut v = Vec::new(); - - self.signature.encode_to(&mut v); - self.function.encode_to(&mut v); - - // need to prefix with the total length to ensure it's binary comptible with - // Vec. - let mut length: Vec<()> = Vec::new(); - length.resize(v.len(), ()); - length.using_encoded(|s| { v.splice(0..0, s.iter().cloned()); }); - - v + super::encode_with_vec_prefix::(|v| { + self.signature.encode_to(v); + self.function.encode_to(v); + }) } } diff --git a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs index e45c778ff3e6b..b5afc165e836f 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs @@ -141,27 +141,19 @@ where Call: Encode, { fn encode(&self) -> Vec { - let mut v = Vec::new(); - - // 1 byte version id. - match self.signature.as_ref() { - Some(s) => { - v.push(TRANSACTION_VERSION | 0b1000_0000); - s.encode_to(&mut v); - } - None => { - v.push(TRANSACTION_VERSION & 0b0111_1111); + super::encode_with_vec_prefix::(|mut v| { + // 1 byte version id. + match self.signature.as_ref() { + Some(s) => { + v.push(TRANSACTION_VERSION | 0b1000_0000); + s.encode_to(&mut v); + } + None => { + v.push(TRANSACTION_VERSION & 0b0111_1111); + } } - } - self.function.encode_to(&mut v); - - // need to prefix with the total length to ensure it's binary comptible with - // Vec. - let mut length: Vec<()> = Vec::new(); - length.resize(v.len(), ()); - length.using_encoded(|s| { v.splice(0..0, s.iter().cloned()); }); - - v + self.function.encode_to(&mut v); + }) } } From d60ed137c573a2696c9b7aa1f553ee30e744c2f2 Mon Sep 17 00:00:00 2001 From: arkpar Date: Tue, 25 Sep 2018 17:43:50 +0200 Subject: [PATCH 3/3] Fixed no-std build --- core/sr-primitives/src/generic/mod.rs | 3 ++- .../sr-primitives/src/generic/unchecked_mortal_extrinsic.rs | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/core/sr-primitives/src/generic/mod.rs b/core/sr-primitives/src/generic/mod.rs index 85d89a38ace28..2f944be29dc39 100644 --- a/core/sr-primitives/src/generic/mod.rs +++ b/core/sr-primitives/src/generic/mod.rs @@ -37,9 +37,10 @@ pub use self::block::{Block, SignedBlock, BlockId}; pub use self::digest::{Digest, DigestItem, DigestItemRef}; use codec::Encode; +use rstd::prelude::*; fn encode_with_vec_prefix)>(encoder: F) -> Vec { - let size = ::std::mem::size_of::(); + let size = ::rstd::mem::size_of::(); let reserve = match size { 0...0b00111111 => 1, 0...0b00111111_11111111 => 2, diff --git a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs index b5afc165e836f..793532c0749fe 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs @@ -141,18 +141,18 @@ where Call: Encode, { fn encode(&self) -> Vec { - super::encode_with_vec_prefix::(|mut v| { + super::encode_with_vec_prefix::(|v| { // 1 byte version id. match self.signature.as_ref() { Some(s) => { v.push(TRANSACTION_VERSION | 0b1000_0000); - s.encode_to(&mut v); + s.encode_to(v); } None => { v.push(TRANSACTION_VERSION & 0b0111_1111); } } - self.function.encode_to(&mut v); + self.function.encode_to(v); }) } }