From bb014eea2512018ce43053a11105a7c35a9358c9 Mon Sep 17 00:00:00 2001 From: joshieDo <93316087+joshieDo@users.noreply.github.com> Date: Fri, 2 Dec 2022 17:56:08 +0800 Subject: [PATCH] codecs: "type specialization" for `Compact` on `Vec` & `Option` (#297) * maybe_zero no longer necessary on bytecode_hash * add alt impl for Compact to/from * add some more docs * add default vec impl on generator test * use default trait impl * rename from alternative to specialized --- crates/codecs/derive/src/compact/enums.rs | 19 +++- crates/codecs/derive/src/compact/flags.rs | 2 +- crates/codecs/derive/src/compact/generator.rs | 2 +- crates/codecs/derive/src/compact/mod.rs | 52 +++++++++-- crates/codecs/derive/src/compact/structs.rs | 26 ++++-- crates/codecs/src/lib.rs | 91 ++++++++++++++++++- crates/primitives/src/account.rs | 1 - 7 files changed, 165 insertions(+), 28 deletions(-) diff --git a/crates/codecs/derive/src/compact/enums.rs b/crates/codecs/derive/src/compact/enums.rs index 62c4ac47aa..978d98d86c 100644 --- a/crates/codecs/derive/src/compact/enums.rs +++ b/crates/codecs/derive/src/compact/enums.rs @@ -56,15 +56,20 @@ impl<'a> EnumHandler<'a> { if let Some(next_field) = self.fields_iterator.peek() { match next_field { - FieldTypes::EnumUnnamedField(next_ftype) => { + FieldTypes::EnumUnnamedField((next_ftype, use_alt_impl)) => { // This variant is of the type `EnumVariant(UnnamedField)` let field_type = format_ident!("{next_ftype}"); + let from_compact_ident = if !use_alt_impl { + format_ident!("from_compact") + } else { + format_ident!("specialized_from_compact") + }; // Unamed type self.enum_lines.push(quote! { #current_variant_index => { let mut inner = #field_type::default(); - (inner, buf) = #field_type::from_compact(buf, buf.len()); + (inner, buf) = #field_type::#from_compact_ident(buf, buf.len()); #ident::#variant_name(inner) } }); @@ -94,11 +99,17 @@ impl<'a> EnumHandler<'a> { if let Some(next_field) = self.fields_iterator.peek() { match next_field { - FieldTypes::EnumUnnamedField(_) => { + FieldTypes::EnumUnnamedField((_, use_alt_impl)) => { + let to_compact_ident = if !use_alt_impl { + format_ident!("to_compact") + } else { + format_ident!("specialized_to_compact") + }; + // Unamed type self.enum_lines.push(quote! { #ident::#variant_name(field) => { - field.to_compact(&mut buffer); + field.#to_compact_ident(&mut buffer); #current_variant_index }, }); diff --git a/crates/codecs/derive/src/compact/flags.rs b/crates/codecs/derive/src/compact/flags.rs index 56ec97aad7..8868236918 100644 --- a/crates/codecs/derive/src/compact/flags.rs +++ b/crates/codecs/derive/src/compact/flags.rs @@ -70,7 +70,7 @@ fn build_struct_field_flags( let mut total_bits = 0; // Find out the adequate bit size for the length of each field, if applicable. - for (name, ftype, is_compact) in fields { + for (name, ftype, is_compact, _) in fields { // This happens when dealing with a wrapper struct eg. Struct(pub U256). let name = if name.is_empty() { "placeholder" } else { name }; diff --git a/crates/codecs/derive/src/compact/generator.rs b/crates/codecs/derive/src/compact/generator.rs index cd9771aa1e..cc65ea1611 100644 --- a/crates/codecs/derive/src/compact/generator.rs +++ b/crates/codecs/derive/src/compact/generator.rs @@ -82,7 +82,7 @@ fn generate_from_compact(fields: &FieldList, ident: &Ident) -> Vec }); } else { let fields = fields.iter().filter_map(|field| { - if let FieldTypes::StructField((name, _, _)) = field { + if let FieldTypes::StructField((name, _, _, _)) = field { let ident = format_ident!("{name}"); return Some(quote! { #ident: #ident, diff --git a/crates/codecs/derive/src/compact/mod.rs b/crates/codecs/derive/src/compact/mod.rs index 3801322173..716efee6f1 100644 --- a/crates/codecs/derive/src/compact/mod.rs +++ b/crates/codecs/derive/src/compact/mod.rs @@ -22,8 +22,14 @@ type IsCompact = bool; type FieldName = String; // Helper Alias type type FieldType = String; +/// `Compact` has alternative functions that can be used as a workaround for type +/// specialization of fixed sized types. +/// +/// Example: `Vec` vs `Vec`. The first does not +/// require the len of the element, while the latter one does. +type UseAlternative = bool; // Helper Alias type -type StructFieldDescriptor = (FieldName, FieldType, IsCompact); +type StructFieldDescriptor = (FieldName, FieldType, IsCompact, UseAlternative); // Helper Alias type type FieldList = Vec; @@ -31,7 +37,7 @@ type FieldList = Vec; pub enum FieldTypes { StructField(StructFieldDescriptor), EnumVariant(String), - EnumUnnamedField(FieldType), + EnumUnnamedField((FieldType, UseAlternative)), } /// Derives the [`Compact`] trait and its from/to implementations. @@ -96,15 +102,20 @@ fn load_field(field: &syn::Field, fields: &mut FieldList, is_enum: bool) { let segments = &path.path.segments; if !segments.is_empty() { let mut ftype = String::new(); + + let mut use_alt_impl: UseAlternative = false; + for (index, segment) in segments.iter().enumerate() { ftype.push_str(&segment.ident.to_string()); if index < segments.len() - 1 { ftype.push_str("::"); } + + use_alt_impl = should_use_alt_impl(&ftype, segment); } if is_enum { - fields.push(FieldTypes::EnumUnnamedField(ftype.to_string())); + fields.push(FieldTypes::EnumUnnamedField((ftype.to_string(), use_alt_impl))); } else { let should_compact = is_flag_type(&ftype) || field.attrs.iter().any(|attr| { @@ -115,12 +126,35 @@ fn load_field(field: &syn::Field, fields: &mut FieldList, is_enum: bool) { field.ident.as_ref().map(|i| i.to_string()).unwrap_or_default(), ftype, should_compact, + use_alt_impl, ))); } } } } +/// Since there's no impl specialization in rust stable atm, once we find we have a +/// Vec/Option we try to find out if it's a Vec/Option of a fixed size data type. +/// eg, Vec. If so, we use another impl to code/decode its data. +fn should_use_alt_impl(ftype: &String, segment: &syn::PathSegment) -> bool { + if *ftype == "Vec" || *ftype == "Option" { + if let syn::PathArguments::AngleBracketed(ref args) = segment.arguments { + if let Some(syn::GenericArgument::Type(syn::Type::Path(arg_path))) = args.args.last() { + if let (Some(path), 1) = + (arg_path.path.segments.first(), arg_path.path.segments.len()) + { + if ["H256", "H160", "Address", "Bloom"] + .contains(&path.ident.to_string().as_str()) + { + return true + } + } + } + } + } + false +} + /// Given the field type in a string format, return the amount of bits necessary to save its maximum /// length. pub fn get_bit_size(ftype: &str) -> u8 { @@ -154,10 +188,10 @@ mod tests { f_u256: U256, f_bool_t: bool, f_bool_f: bool, - f_option_none: Option, + f_option_none: Option, f_option_some: Option, f_option_some_u64: Option, - f_vec_empty: Vec, + f_vec_empty: Vec, f_vec_some: Vec, } }; @@ -220,12 +254,12 @@ mod tests { flags.set_f_bool_f_len(f_bool_f_len as u8); let f_option_none_len = self.f_option_none.to_compact(&mut buffer); flags.set_f_option_none_len(f_option_none_len as u8); - let f_option_some_len = self.f_option_some.to_compact(&mut buffer); + let f_option_some_len = self.f_option_some.specialized_to_compact(&mut buffer); flags.set_f_option_some_len(f_option_some_len as u8); let f_option_some_u64_len = self.f_option_some_u64.to_compact(&mut buffer); flags.set_f_option_some_u64_len(f_option_some_u64_len as u8); let f_vec_empty_len = self.f_vec_empty.to_compact(&mut buffer); - let f_vec_some_len = self.f_vec_some.to_compact(&mut buffer); + let f_vec_some_len = self.f_vec_some.specialized_to_compact(&mut buffer); let flags = flags.into_bytes(); total_len += flags.len() + buffer.len(); buf.put_slice(&flags); @@ -245,14 +279,14 @@ mod tests { let mut f_option_none = Option::default(); (f_option_none, buf) = Option::from_compact(buf, flags.f_option_none_len() as usize); let mut f_option_some = Option::default(); - (f_option_some, buf) = Option::from_compact(buf, flags.f_option_some_len() as usize); + (f_option_some, buf) = Option::specialized_from_compact(buf, flags.f_option_some_len() as usize); let mut f_option_some_u64 = Option::default(); (f_option_some_u64, buf) = Option::from_compact(buf, flags.f_option_some_u64_len() as usize); let mut f_vec_empty = Vec::default(); (f_vec_empty, buf) = Vec::from_compact(buf, buf.len()); let mut f_vec_some = Vec::default(); - (f_vec_some, buf) = Vec::from_compact(buf, buf.len()); + (f_vec_some, buf) = Vec::specialized_from_compact(buf, buf.len()); let obj = TestStruct { f_u64: f_u64, f_u256: f_u256, diff --git a/crates/codecs/derive/src/compact/structs.rs b/crates/codecs/derive/src/compact/structs.rs index aee19937ef..dfa1e9169c 100644 --- a/crates/codecs/derive/src/compact/structs.rs +++ b/crates/codecs/derive/src/compact/structs.rs @@ -46,14 +46,20 @@ impl<'a> StructHandler<'a> { /// Generates `to_compact` code for a struct field. fn to(&mut self, field_descriptor: &StructFieldDescriptor) { - let (name, ftype, is_compact) = field_descriptor; + let (name, ftype, is_compact, use_alt_impl) = field_descriptor; + + let to_compact_ident = if !use_alt_impl { + format_ident!("to_compact") + } else { + format_ident!("specialized_to_compact") + }; // Should only happen on wrapper structs like `Struct(pub Field)` if name.is_empty() { self.is_wrapper = true; self.lines.push(quote! { - let _len = self.0.to_compact(&mut buffer); + let _len = self.0.#to_compact_ident(&mut buffer); }); if is_flag_type(ftype) { @@ -76,12 +82,12 @@ impl<'a> StructHandler<'a> { self.lines.push(quote! { if self.#name != #itype::zero() { flags.#set_bool_method(true); - self.#name.to_compact(&mut buffer); + self.#name.#to_compact_ident(&mut buffer); }; }); } else { self.lines.push(quote! { - let #len = self.#name.to_compact(&mut buffer); + let #len = self.#name.#to_compact_ident(&mut buffer); }); } if is_flag_type(ftype) { @@ -93,7 +99,7 @@ impl<'a> StructHandler<'a> { /// Generates `from_compact` code for a struct field. fn from(&mut self, field_descriptor: &StructFieldDescriptor, known_types: &[&str]) { - let (name, ftype, is_compact) = field_descriptor; + let (name, ftype, is_compact, use_alt_impl) = field_descriptor; let (name, len) = if name.is_empty() { self.is_wrapper = true; @@ -104,6 +110,12 @@ impl<'a> StructHandler<'a> { (format_ident!("{name}"), format_ident!("{name}_len")) }; + let from_compact_ident = if !use_alt_impl { + format_ident!("from_compact") + } else { + format_ident!("specialized_from_compact") + }; + assert!( known_types.contains(&ftype.as_str()) || is_flag_type(ftype) || @@ -125,11 +137,11 @@ impl<'a> StructHandler<'a> { if !is_flag_type(ftype) { // It's a type that handles its own length requirements. (h256, Custom, ...) self.lines.push(quote! { - (#name, buf) = #ident_type::from_compact(buf, buf.len()); + (#name, buf) = #ident_type::#from_compact_ident(buf, buf.len()); }) } else if *is_compact { self.lines.push(quote! { - (#name, buf) = #ident_type::from_compact(buf, flags.#len() as usize); + (#name, buf) = #ident_type::#from_compact_ident(buf, flags.#len() as usize); }); } else { todo!() diff --git a/crates/codecs/src/lib.rs b/crates/codecs/src/lib.rs index 4b49dae4b8..4a867dac76 100644 --- a/crates/codecs/src/lib.rs +++ b/crates/codecs/src/lib.rs @@ -9,11 +9,16 @@ use ethers_core::types::{Bloom, H160, H256, U256}; /// * Fixed array types (H256, Address, Bloom) are not compacted. /// * Max size of `T` in `Option` or `Vec` shouldn't exceed `0xffff`. /// * Any `bytes::Bytes` field **should be placed last**. -/// * Any other type which is not known to the derive module **should be placed last**. +/// * Any other type which is not known to the derive module **should be placed last** in they +/// contain a `bytes::Bytes` field. /// /// The last two points make it easier to decode the data without saving the length on the /// `StructFlags`. It will fail compilation if it's not respected. If they're alias to known types, /// add their definitions to `get_bit_size()` or `known_types` in `generator.rs`. +/// +/// Regarding the `specialized_to/from_compact` methods: Mainly used as a workaround for not being +/// able to specialize an impl over certain types like Vec/Option where T is a fixed size +/// array like Vec. pub trait Compact { /// Takes a buffer which can be written to. *Ideally*, it returns the length written to. fn to_compact(self, buf: &mut impl bytes::BufMut) -> usize; @@ -26,6 +31,22 @@ pub trait Compact { fn from_compact(buf: &[u8], len: usize) -> (Self, &[u8]) where Self: Sized; + + /// "Optional": If there's no good reason to use it, don't. + fn specialized_to_compact(self, buf: &mut impl bytes::BufMut) -> usize + where + Self: Sized, + { + self.to_compact(buf) + } + + /// "Optional": If there's no good reason to use it, don't. + fn specialized_from_compact(buf: &[u8], len: usize) -> (Self, &[u8]) + where + Self: Sized, + { + Self::from_compact(buf, len) + } } macro_rules! impl_uint_compact { @@ -89,6 +110,32 @@ where (list, buf) } + + /// To be used by fixed sized types like Vec. + fn specialized_to_compact(self, buf: &mut impl bytes::BufMut) -> usize { + buf.put_u16(self.len() as u16); + + for element in self { + element.to_compact(buf); + } + 0 + } + + /// To be used by fixed sized types like Vec. + fn specialized_from_compact(mut buf: &[u8], len: usize) -> (Self, &[u8]) { + let mut list = vec![]; + let length = buf.get_u16(); + for _ in 0..length { + #[allow(unused_assignments)] + let mut element = T::default(); + + (element, buf) = T::from_compact(buf, len); + + list.push(element); + } + + (list, buf) + } } impl Compact for Option @@ -117,6 +164,26 @@ where (Some(element), buf) } + + /// To be used by fixed sized types like Option. + fn specialized_to_compact(self, buf: &mut impl bytes::BufMut) -> usize { + if let Some(element) = self { + element.to_compact(buf); + return 1 + } + 0 + } + + /// To be used by fixed sized types like Option. + fn specialized_from_compact(buf: &[u8], len: usize) -> (Self, &[u8]) { + if len == 0 { + return (None, buf) + } + + let (element, buf) = T::from_compact(buf, len); + + (Some(element), buf) + } } impl Compact for U256 { @@ -171,6 +238,14 @@ macro_rules! impl_hash_compact { buf.advance(std::mem::size_of::<$name>()); (v, buf) } + + fn specialized_to_compact(self, buf: &mut impl bytes::BufMut) -> usize { + self.to_compact(buf) + } + + fn specialized_from_compact(buf: &[u8], len: usize) -> (Self, &[u8]) { + Self::from_compact(buf, len) + } } )+ }; @@ -288,11 +363,17 @@ mod tests { assert_eq!(None::.to_compact(&mut buf), 0); assert_eq!(opt.to_compact(&mut buf), 1); + assert_eq!(buf.len(), 34); assert_eq!(Option::::from_compact(&buf, 1), (opt, vec![].as_slice())); // If `None`, it returns the slice at the same cursor position. assert_eq!(Option::::from_compact(&buf, 0), (None, buf.as_slice())); + + let mut buf = vec![]; + assert_eq!(opt.specialized_to_compact(&mut buf), 1); + assert_eq!(buf.len(), 32); + assert_eq!(Option::::specialized_from_compact(&buf, 1), (opt, vec![].as_slice())); } #[test] @@ -367,10 +448,10 @@ mod tests { f_bool_f: false, // 1 bit | 0 bytes f_bool_t: true, // 1 bit | 0 bytes f_option_none: None, // 1 bit | 0 bytes - f_option_some: Some(H256::zero()), // 1 bit | 2 + 32 bytes + f_option_some: Some(H256::zero()), // 1 bit | 32 bytes f_option_some_u64: Some(0xffffu64), // 1 bit | 2 + 2 bytes f_vec_empty: vec![], // 0 bits | 2 bytes - f_vec_some: vec![H160::zero(), H160::zero()], // 0 bits | 2 + (2+20)*2 bytes + f_vec_some: vec![H160::zero(), H160::zero()], // 0 bits | 2 + 20*2 bytes } } } @@ -385,10 +466,10 @@ mod tests { 1 + 1 + // 0 + 0 + 0 + - 2 + 32 + + 32 + 2 + 2 + 2 + - 2 + (2 + 20) * 2 + 2 + 20 * 2 ); assert_eq!( diff --git a/crates/primitives/src/account.rs b/crates/primitives/src/account.rs index 1850b32d0c..20062d1c4d 100644 --- a/crates/primitives/src/account.rs +++ b/crates/primitives/src/account.rs @@ -11,7 +11,6 @@ pub struct Account { pub nonce: u64, /// Account balance. pub balance: U256, - #[maybe_zero] /// Hash of the bytecode. pub bytecode_hash: Option, }