From 99e234e00a9f44673c35949d16fb36c1a9559b2b Mon Sep 17 00:00:00 2001 From: Filippo Neysofu Costa Date: Tue, 15 Aug 2023 13:06:57 +0200 Subject: [PATCH] Allow custom (de)serialization logic for `#[state]` (#648) * Allow custom state encodings * Use Default::default to build codecs * Improve tests for sov-modules-macros * Stricter type bounds for better error messages * Don't reexport sov_state::codec * t.pass test --- Cargo.lock | 1 + module-system/sov-modules-macros/Cargo.toml | 3 +- .../sov-modules-macros/src/module_info.rs | 532 +++++++++++------- .../sov-modules-macros/tests/all_tests.rs | 3 +- .../tests/custom_codec_must_be_used.rs | 46 ++ .../tests/module_info/custom_codec_builder.rs | 16 + .../module_info/not_supported_attribute.rs | 5 +- .../not_supported_attribute.stderr | 11 - .../second_addr_not_supported.stderr | 10 +- module-system/sov-state/src/codec.rs | 156 +++++ module-system/sov-state/src/lib.rs | 3 +- module-system/sov-state/src/map.rs | 68 ++- module-system/sov-state/src/prover_storage.rs | 6 +- module-system/sov-state/src/scratchpad.rs | 55 +- module-system/sov-state/src/storage.rs | 17 +- module-system/sov-state/src/value.rs | 107 +++- 16 files changed, 736 insertions(+), 303 deletions(-) create mode 100644 module-system/sov-modules-macros/tests/custom_codec_must_be_used.rs create mode 100644 module-system/sov-modules-macros/tests/module_info/custom_codec_builder.rs delete mode 100644 module-system/sov-modules-macros/tests/module_info/not_supported_attribute.stderr create mode 100644 module-system/sov-state/src/codec.rs diff --git a/Cargo.lock b/Cargo.lock index fc9a910de..37d629c24 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6731,6 +6731,7 @@ dependencies = [ "sov-modules-api", "sov-state", "syn 1.0.109", + "tempfile", "trybuild", ] diff --git a/module-system/sov-modules-macros/Cargo.toml b/module-system/sov-modules-macros/Cargo.toml index f75d180ad..2df88f288 100644 --- a/module-system/sov-modules-macros/Cargo.toml +++ b/module-system/sov-modules-macros/Cargo.toml @@ -21,7 +21,8 @@ path = "tests/all_tests.rs" [dev-dependencies] serde_json = "1" -jsonrpsee = { workspace = true, features = ["macros", "http-client", "server"]} +tempfile = "3" +jsonrpsee = { workspace = true, features = ["macros", "http-client", "server"] } trybuild = "1.0" sov-modules-api = { path = "../sov-modules-api", version = "0.1", default-features = false } diff --git a/module-system/sov-modules-macros/src/module_info.rs b/module-system/sov-modules-macros/src/module_info.rs index 982315512..cca82959e 100644 --- a/module-system/sov-modules-macros/src/module_info.rs +++ b/module-system/sov-modules-macros/src/module_info.rs @@ -1,59 +1,18 @@ use proc_macro2::{self, Ident, Span}; -use syn::{DataStruct, DeriveInput, ImplGenerics, PathArguments, TypeGenerics, WhereClause}; +use syn::{ + Attribute, DataStruct, DeriveInput, ImplGenerics, PathArguments, TypeGenerics, WhereClause, +}; +use self::parsing::{ModuleField, ModuleFieldAttribute, StructDef}; use crate::common::get_generics_type_param; -#[derive(Clone)] -struct StructNamedField { - ident: proc_macro2::Ident, - ty: syn::Type, -} - -// A field can be either a state variable or another module. -// We don't generate prefix functions for imported modules as they are already generated. -#[derive(Clone)] -enum FieldKind { - Address(StructNamedField), - State(StructNamedField), - Module(StructNamedField), -} - -struct StructDef<'a> { - ident: proc_macro2::Ident, - impl_generics: ImplGenerics<'a>, - type_generics: TypeGenerics<'a>, - generic_param: &'a Ident, - - fields: Result, syn::Error>, - where_clause: Option<&'a WhereClause>, -} - pub(crate) fn derive_module_info( input: DeriveInput, ) -> Result { - let DeriveInput { - data, - ident, - generics, - .. - } = input; - - let generic_param = get_generics_type_param(&generics, Span::call_site())?; - - let (impl_generics, type_generics, where_clause) = generics.split_for_impl(); - let fields = get_fields_from_struct(&data); - - let struct_def = StructDef { - ident, - fields, - impl_generics, - type_generics, - generic_param: &generic_param, - where_clause, - }; + let struct_def = StructDef::parse(&input)?; - let impl_prefix_functions = struct_def.impl_prefix_functions()?; - let impl_new = struct_def.impl_module_info()?; + let impl_prefix_functions = impl_prefix_functions(&struct_def)?; + let impl_new = impl_module_info(&struct_def)?; Ok(quote::quote! { #impl_prefix_functions @@ -63,162 +22,102 @@ pub(crate) fn derive_module_info( .into()) } -impl<'a> StructDef<'a> { - // Creates a prefix function for each field of the underlying structure. - fn impl_prefix_functions(&self) -> Result { - let fields = self.fields.clone()?; - - let impl_prefix_functions = fields.iter().filter_map(|field| match field { - FieldKind::State(field) => Some(make_prefix_func(field, &self.ident)), - // Don't generate prefix functions for modules - FieldKind::Module(_) => None, - // Don't generate prefix functions for address - FieldKind::Address(_) => None, - }); - - let impl_generics = &self.impl_generics; - let ident = &self.ident; - let ty_generics = &self.type_generics; - let where_clause = self.where_clause; - - Ok(quote::quote! { - impl #impl_generics #ident #ty_generics #where_clause{ - #(#impl_prefix_functions)* - } - }) - } - - // Implements the `ModuleInfo` trait. - fn impl_module_info(&self) -> Result { - let fields = self.fields.clone()?; - let type_generics = &self.type_generics; - - let mut impl_self_init = Vec::default(); - let mut impl_self_body = Vec::default(); - let mut modules = Vec::default(); - - let mut module_address = None; - for field in fields.iter() { - match field { - FieldKind::State(field) => { - impl_self_init.push(make_init_state(field)?); - impl_self_body.push(&field.ident); - } - FieldKind::Module(field) => { - impl_self_init.push(make_init_module(field)?); - impl_self_body.push(&field.ident); - modules.push(&field.ident); - } - FieldKind::Address(field) => { - impl_self_init.push(make_init_address( - field, - &self.ident, - module_address, - self.generic_param, - )?); - impl_self_body.push(&field.ident); - module_address = Some(&field.ident); - } - }; - } - - let generic_param = self.generic_param; - let impl_generics = &self.impl_generics; - let ident = &self.ident; - - let where_clause = self.where_clause; +// Creates a prefix function for each field of the underlying structure. +fn impl_prefix_functions(struct_def: &StructDef) -> Result { + let StructDef { + ident, + impl_generics, + type_generics, + fields, + where_clause, + .. + } = struct_def; - let fn_address = make_fn_address(module_address)?; - let fn_dependencies = make_fn_dependencies(modules); + let prefix_functions = fields + .iter() + // Don't generate prefix functions for modules or addresses; only state. + .filter(|field| matches!(field.attr, ModuleFieldAttribute::State { .. })) + .map(|field| make_prefix_func(field, ident)); - Ok(quote::quote! { - impl #impl_generics ::std::default::Default for #ident #type_generics #where_clause{ + Ok(quote::quote! { + impl #impl_generics #ident #type_generics #where_clause{ + #(#prefix_functions)* + } + }) +} - fn default() -> Self { - #(#impl_self_init)* +// Implements the `ModuleInfo` trait. +fn impl_module_info(struct_def: &StructDef) -> Result { + let module_address = struct_def.module_address(); - Self{ - #(#impl_self_body),* - } - } + let StructDef { + ident, + impl_generics, + type_generics, + generic_param, + fields, + where_clause, + } = struct_def; + + let mut impl_self_init = Vec::default(); + let mut impl_self_body = Vec::default(); + let mut modules = Vec::default(); + + for field in fields.iter() { + match &field.attr { + ModuleFieldAttribute::State { codec_builder } => { + impl_self_init.push(make_init_state( + field, + &codec_builder + .as_ref() + .cloned() + .unwrap_or_else(default_codec_builder), + )?); + impl_self_body.push(&field.ident); } - - impl #impl_generics ::sov_modules_api::ModuleInfo for #ident #type_generics #where_clause{ - type Context = #generic_param; - - #fn_address - - #fn_dependencies + ModuleFieldAttribute::Module => { + impl_self_init.push(make_init_module(field)?); + impl_self_body.push(&field.ident); + modules.push(&field.ident); } - }) + ModuleFieldAttribute::Address => { + impl_self_init.push(make_init_address(field, ident, generic_param)?); + impl_self_body.push(&field.ident); + } + }; } -} -// Extracts named fields form a struct or emits an error. -fn get_fields_from_struct(data: &syn::Data) -> Result, syn::Error> { - match data { - syn::Data::Struct(data_struct) => get_fields_from_data_struct(data_struct), - syn::Data::Enum(en) => Err(syn::Error::new_spanned( - en.enum_token, - "The `ModuleInfo` macro supports structs only.", - )), - syn::Data::Union(un) => Err(syn::Error::new_spanned( - un.union_token, - "The `ModuleInfo` macro supports structs only.", - )), - } -} + let fn_address = make_fn_address(&module_address.ident)?; + let fn_dependencies = make_fn_dependencies(modules); -fn get_fields_from_data_struct(data_struct: &DataStruct) -> Result, syn::Error> { - let mut output_fields = Vec::default(); + Ok(quote::quote! { + impl #impl_generics ::std::default::Default for #ident #type_generics #where_clause{ - for original_field in data_struct.fields.iter() { - let field_ident = original_field - .ident - .as_ref() - .ok_or(syn::Error::new_spanned( - &original_field.ident, - "The `ModuleInfo` macro supports structs only, unnamed fields witnessed.", - ))?; + fn default() -> Self { + #(#impl_self_init)* - if original_field.attrs.is_empty() { - return Err(syn::Error::new_spanned( - &original_field.ident, - "This field is missing an attribute: add `#[module]`, `#[state]` or `#[address]`. ", - )); + Self{ + #(#impl_self_body),* + } + } } - for attribute in &original_field.attrs { - let field = StructNamedField { - ident: field_ident.clone(), - ty: original_field.ty.clone(), - }; - - if attribute.path.segments[0].ident == "state" { - output_fields.push(FieldKind::State(field)); - } else if attribute.path.segments[0].ident == "module" { - output_fields.push(FieldKind::Module(field)) - } else if attribute.path.segments[0].ident == "address" { - output_fields.push(FieldKind::Address(field)) - } else if attribute.path.segments[0].ident == "doc" { - // Skip doc comments. - } else { - return Err(syn::Error::new_spanned( - field_ident, - "Only `#[module]`, `#[state]` or `#[address]` attributes are supported.", - )); - }; + impl #impl_generics ::sov_modules_api::ModuleInfo for #ident #type_generics #where_clause{ + type Context = #generic_param; + + #fn_address + + #fn_dependencies } - } - Ok(output_fields) + }) } -fn prefix_func_ident(ident: &proc_macro2::Ident) -> proc_macro2::Ident { - syn::Ident::new(&format!("_prefix_{ident}"), ident.span()) +fn default_codec_builder() -> syn::Path { + syn::parse_str("::core::default::Default::default").unwrap() } fn make_prefix_func( - field: &StructNamedField, + field: &ModuleField, module_ident: &proc_macro2::Ident, ) -> proc_macro2::TokenStream { let field_ident = &field.ident; @@ -237,20 +136,18 @@ fn make_prefix_func( } } +fn prefix_func_ident(ident: &proc_macro2::Ident) -> proc_macro2::Ident { + syn::Ident::new(&format!("_prefix_{ident}"), ident.span()) +} + fn make_fn_address( - address_ident: Option<&proc_macro2::Ident>, + address_ident: &proc_macro2::Ident, ) -> Result { - match address_ident { - Some(address_ident) => Ok(quote::quote! { - fn address(&self) -> &::Address { - &self.#address_ident - } - }), - None => Err(syn::Error::new( - Span::call_site(), - "The `ModuleInfo` macro requires `[address]` attribute.", - )), - } + Ok(quote::quote! { + fn address(&self) -> &::Address { + &self.#address_ident + } + }) } fn make_fn_dependencies(modules: Vec<&proc_macro2::Ident>) -> proc_macro2::TokenStream { @@ -266,7 +163,10 @@ fn make_fn_dependencies(modules: Vec<&proc_macro2::Ident>) -> proc_macro2::Token } } } -fn make_init_state(field: &StructNamedField) -> Result { +fn make_init_state( + field: &ModuleField, + encoding_constructor: &syn::Path, +) -> Result { let prefix_fun = prefix_func_ident(&field.ident); let field_ident = &field.ident; let ty = &field.ty; @@ -297,11 +197,11 @@ fn make_init_state(field: &StructNamedField) -> Result Result { +fn make_init_module(field: &ModuleField) -> Result { let field_ident = &field.ident; let ty = &field.ty; @@ -311,28 +211,224 @@ fn make_init_module(field: &StructNamedField) -> Result, generic_param: &Ident, ) -> Result { let field_ident = &field.ident; - match address { - Some(addr) => Err(syn::Error::new_spanned( - addr, - format!( - "The `address` attribute is defined more than once, revisit field: {}", - addr - ), - )), - None => Ok(quote::quote! { - use ::sov_modules_api::digest::Digest as _; - let module_path = module_path!(); - let prefix = sov_modules_api::Prefix::new_module(module_path, stringify!(#struct_ident)); - let #field_ident : <#generic_param as sov_modules_api::Spec>::Address = - <#generic_param as ::sov_modules_api::Spec>::Address::try_from(&prefix.hash::<#generic_param>()) - .unwrap_or_else(|e| panic!("ModuleInfo macro error, unable to create an Address for module: {}", e)); - }), + Ok(quote::quote! { + use ::sov_modules_api::digest::Digest as _; + let module_path = module_path!(); + let prefix = sov_modules_api::Prefix::new_module(module_path, stringify!(#struct_ident)); + let #field_ident : <#generic_param as sov_modules_api::Spec>::Address = + <#generic_param as ::sov_modules_api::Spec>::Address::try_from(&prefix.hash::<#generic_param>()) + .unwrap_or_else(|e| panic!("ModuleInfo macro error, unable to create an Address for module: {}", e)); + }) +} + +/// Internal `proc macro` parsing utilities. +pub mod parsing { + use super::*; + + pub struct StructDef<'a> { + pub ident: &'a proc_macro2::Ident, + pub impl_generics: ImplGenerics<'a>, + pub type_generics: TypeGenerics<'a>, + pub generic_param: Ident, + + pub fields: Vec, + pub where_clause: Option<&'a WhereClause>, + } + + impl<'a> StructDef<'a> { + pub fn parse(input: &'a DeriveInput) -> syn::Result { + let ident = &input.ident; + let generic_param = get_generics_type_param(&input.generics, Span::call_site())?; + let (impl_generics, type_generics, where_clause) = input.generics.split_for_impl(); + let fields = parse_module_fields(&input.data)?; + check_exactly_one_address(&fields)?; + + Ok(StructDef { + ident, + fields, + impl_generics, + type_generics, + generic_param, + where_clause, + }) + } + + pub fn module_address(&self) -> &ModuleField { + self.fields + .iter() + .find(|field| matches!(field.attr, ModuleFieldAttribute::Address)) + .expect("Module address not found but it was validated already; this is a bug") + } + } + + #[derive(Clone)] + pub struct ModuleField { + pub ident: syn::Ident, + pub ty: syn::Type, + pub attr: ModuleFieldAttribute, + } + + #[derive(Clone)] + pub enum ModuleFieldAttribute { + Module, + State { codec_builder: Option }, + Address, + } + + impl ModuleFieldAttribute { + fn parse(attr: &Attribute) -> syn::Result { + match attr.path.segments[0].ident.to_string().as_str() { + "module" => { + if attr.tokens.is_empty() { + Ok(Self::Module) + } else { + Err(syn::Error::new_spanned( + attr, + "The `#[module]` attribute does not accept any arguments.", + )) + } + } + "address" => { + if attr.tokens.is_empty() { + Ok(Self::Address) + } else { + Err(syn::Error::new_spanned( + attr, + "The `#[address]` attribute does not accept any arguments.", + )) + } + } + "state" => parse_state_attr(attr), + _ => unreachable!("attribute names were validated already; this is a bug"), + } + } + } + + fn parse_state_attr(attr: &Attribute) -> syn::Result { + let syntax_err = + syn::Error::new_spanned(attr, "Invalid syntax for the `#[state]` attribute."); + + let meta = if attr.tokens.is_empty() { + return Ok(ModuleFieldAttribute::State { + codec_builder: None, + }); + } else { + attr.parse_meta()? + }; + + let meta_list = match meta { + syn::Meta::List(l) if !l.nested.is_empty() => l, + _ => return Err(syntax_err), + }; + let name_value = match &meta_list.nested[0] { + syn::NestedMeta::Meta(syn::Meta::NameValue(nv)) => nv, + _ => return Err(syntax_err), + }; + + if name_value.path.get_ident().map(Ident::to_string).as_deref() != Some("codec_builder") { + return Err(syntax_err); + } + + let codec_builder_path = match &name_value.lit { + syn::Lit::Str(lit) => lit.parse_with(syn::Path::parse_mod_style)?, + _ => return Err(syntax_err), + }; + Ok(ModuleFieldAttribute::State { + codec_builder: Some(codec_builder_path), + }) + } + + fn parse_module_fields(data: &syn::Data) -> syn::Result> { + let data_struct = data_to_struct(data)?; + let mut parsed_fields = vec![]; + + for field in data_struct.fields.iter() { + let ident = get_field_ident(field)?; + let ty = field.ty.clone(); + let attr = get_field_attribute(field)?; + + parsed_fields.push(ModuleField { + ident: ident.clone(), + ty, + attr: ModuleFieldAttribute::parse(attr)?, + }); + } + + Ok(parsed_fields) + } + + fn check_exactly_one_address(fields: &[ModuleField]) -> syn::Result<()> { + let address_fields = fields + .iter() + .filter(|field| matches!(field.attr, ModuleFieldAttribute::Address)) + .collect::>(); + + match address_fields.len() { + 0 => Err(syn::Error::new( + Span::call_site(), + "The `ModuleInfo` macro requires `[address]` attribute.", + )), + 1 => Ok(()), + _ => Err(syn::Error::new_spanned( + address_fields[1].ident.clone(), + format!( + "The `address` attribute is defined more than once, revisit field: {}", + address_fields[1].ident, + ), + )), + } + } + + fn data_to_struct(data: &syn::Data) -> syn::Result<&DataStruct> { + match data { + syn::Data::Struct(data_struct) => Ok(data_struct), + syn::Data::Enum(en) => Err(syn::Error::new_spanned( + en.enum_token, + "The `ModuleInfo` macro supports structs only.", + )), + syn::Data::Union(un) => Err(syn::Error::new_spanned( + un.union_token, + "The `ModuleInfo` macro supports structs only.", + )), + } + } + + fn get_field_ident(field: &syn::Field) -> syn::Result<&syn::Ident> { + field.ident.as_ref().ok_or(syn::Error::new_spanned( + field, + "The `ModuleInfo` macro supports structs only, unnamed fields witnessed.", + )) + } + + fn get_field_attribute(field: &syn::Field) -> syn::Result<&Attribute> { + let ident = get_field_ident(field)?; + let mut attr = None; + for a in field.attrs.iter() { + match a.path.segments[0].ident.to_string().as_str() { + "state" | "module" | "address" => { + if attr.is_some() { + return Err(syn::Error::new_spanned(ident, "Only one attribute out of `#[module]`, `#[state]` and `#[address]` is allowed per field.")); + } else { + attr = Some(a); + } + } + _ => {} + } + } + + if let Some(attr) = attr { + Ok(attr) + } else { + Err(syn::Error::new_spanned( + ident, + "This field is missing an attribute: add `#[module]`, `#[state]` or `#[address]`.", + )) + } } } diff --git a/module-system/sov-modules-macros/tests/all_tests.rs b/module-system/sov-modules-macros/tests/all_tests.rs index 30d1240b1..0aa280ca2 100644 --- a/module-system/sov-modules-macros/tests/all_tests.rs +++ b/module-system/sov-modules-macros/tests/all_tests.rs @@ -4,11 +4,12 @@ fn module_info_tests() { t.pass("tests/module_info/parse.rs"); t.pass("tests/module_info/mod_and_state.rs"); t.pass("tests/module_info/use_address_trait.rs"); + t.pass("tests/module_info/not_supported_attribute.rs"); + t.pass("tests/module_info/custom_codec_builder.rs"); t.compile_fail("tests/module_info/derive_on_enum_not_supported.rs"); t.compile_fail("tests/module_info/field_missing_attribute.rs"); t.compile_fail("tests/module_info/missing_address.rs"); t.compile_fail("tests/module_info/no_generics.rs"); - t.compile_fail("tests/module_info/not_supported_attribute.rs"); t.compile_fail("tests/module_info/not_supported_type.rs"); t.compile_fail("tests/module_info/second_addr_not_supported.rs"); } diff --git a/module-system/sov-modules-macros/tests/custom_codec_must_be_used.rs b/module-system/sov-modules-macros/tests/custom_codec_must_be_used.rs new file mode 100644 index 000000000..86f73e0c1 --- /dev/null +++ b/module-system/sov-modules-macros/tests/custom_codec_must_be_used.rs @@ -0,0 +1,46 @@ +use std::panic::catch_unwind; + +use sov_modules_api::default_context::ZkDefaultContext; +use sov_modules_api::{Context, ModuleInfo}; +use sov_state::{DefaultStorageSpec, ProverStorage, StateValue, StateValueCodec, WorkingSet}; + +#[derive(ModuleInfo)] +struct TestModule +where + C: Context, +{ + #[address] + pub address: C::Address, + + #[state(codec_builder = "Default::default")] + pub state_value: StateValue, +} + +#[derive(Default)] +pub struct CustomCodec; + +impl StateValueCodec for CustomCodec { + type ValueError = String; + + fn encode_value(&self, _value: &V) -> Vec { + unimplemented!() + } + + fn try_decode_value(&self, _bytes: &[u8]) -> Result { + unimplemented!() + } +} + +#[test] +fn main() { + let module: TestModule = Default::default(); + + let tempdir = tempfile::tempdir().unwrap(); + let storage: ProverStorage = ProverStorage::with_path(&tempdir).unwrap(); + + catch_unwind(|| { + let mut working_set = WorkingSet::new(storage); + module.state_value.set(&0u32, &mut working_set); + }) + .unwrap_err(); +} diff --git a/module-system/sov-modules-macros/tests/module_info/custom_codec_builder.rs b/module-system/sov-modules-macros/tests/module_info/custom_codec_builder.rs new file mode 100644 index 000000000..e5101a2b2 --- /dev/null +++ b/module-system/sov-modules-macros/tests/module_info/custom_codec_builder.rs @@ -0,0 +1,16 @@ +use sov_modules_api::{Context, ModuleInfo}; +use sov_state::StateMap; + +#[derive(ModuleInfo)] +struct FirstTestStruct +where + C: Context, +{ + #[address] + pub address: C::Address, + + #[state(codec_builder = "sov_state::codec::BorshCodec::default")] + pub state_in_first_struct_1: StateMap, +} + +fn main() {} diff --git a/module-system/sov-modules-macros/tests/module_info/not_supported_attribute.rs b/module-system/sov-modules-macros/tests/module_info/not_supported_attribute.rs index 1f01783ff..a244c7b30 100644 --- a/module-system/sov-modules-macros/tests/module_info/not_supported_attribute.rs +++ b/module-system/sov-modules-macros/tests/module_info/not_supported_attribute.rs @@ -6,7 +6,10 @@ struct TestStruct { #[address] address: C::Address, - #[other] + // Unsupported attributes should be ignored to guarantee compatibility with + // other macros. + #[allow(dead_code)] + #[state] test_state1: StateMap, #[state] diff --git a/module-system/sov-modules-macros/tests/module_info/not_supported_attribute.stderr b/module-system/sov-modules-macros/tests/module_info/not_supported_attribute.stderr deleted file mode 100644 index a36937a55..000000000 --- a/module-system/sov-modules-macros/tests/module_info/not_supported_attribute.stderr +++ /dev/null @@ -1,11 +0,0 @@ -error: Only `#[module]`, `#[state]` or `#[address]` attributes are supported. - --> tests/module_info/not_supported_attribute.rs:10:5 - | -10 | test_state1: StateMap, - | ^^^^^^^^^^^ - -error: cannot find attribute `other` in this scope - --> tests/module_info/not_supported_attribute.rs:9:7 - | -9 | #[other] - | ^^^^^ diff --git a/module-system/sov-modules-macros/tests/module_info/second_addr_not_supported.stderr b/module-system/sov-modules-macros/tests/module_info/second_addr_not_supported.stderr index ed877e3be..b7117973c 100644 --- a/module-system/sov-modules-macros/tests/module_info/second_addr_not_supported.stderr +++ b/module-system/sov-modules-macros/tests/module_info/second_addr_not_supported.stderr @@ -1,5 +1,5 @@ -error: The `address` attribute is defined more than once, revisit field: address_1 - --> tests/module_info/second_addr_not_supported.rs:7:5 - | -7 | address_1: C::Address, - | ^^^^^^^^^ +error: The `address` attribute is defined more than once, revisit field: address_2 + --> tests/module_info/second_addr_not_supported.rs:10:5 + | +10 | address_2: C::Address, + | ^^^^^^^^^ diff --git a/module-system/sov-state/src/codec.rs b/module-system/sov-state/src/codec.rs new file mode 100644 index 000000000..823920abe --- /dev/null +++ b/module-system/sov-state/src/codec.rs @@ -0,0 +1,156 @@ +//! Serialization and deserialization -related logic. + +/// A trait for types that can serialize and deserialize keys for storage +/// access. +pub trait StateKeyCodec { + /// Error type that can arise during deserialization. + type KeyError: std::fmt::Debug; + + /// Serializes a key into a bytes vector. + /// + /// This method **must** not panic as all instances of the key type are + /// supposed to be serializable. + fn encode_key(&self, key: &K) -> Vec; + + /// Tries to deserialize a key from a bytes slice, and returns a + /// [`Result`] with either the deserialized key or an error. + fn try_decode_key(&self, bytes: &[u8]) -> Result; + + /// Deserializes a key from a bytes slice. + /// + /// # Panics + /// Panics if the call to [`StateKeyCodec::try_decode_key`] fails. Use + /// [`StateKeyCodec::try_decode_key`] if you need to gracefully handle + /// errors. + fn decode_key(&self, bytes: &[u8]) -> K { + self.try_decode_key(bytes) + .map_err(|err| { + format!( + "Failed to decode key 0x{}, error: {:?}", + hex::encode(bytes), + err + ) + }) + .unwrap() + } +} + +/// A trait for types that can serialize and deserialize values for storage +/// access. +pub trait StateValueCodec { + /// Error type that can arise during deserialization. + type ValueError: std::fmt::Debug; + + /// Serializes a value into a bytes vector. + /// + /// This method **must** not panic as all instances of the value type are + /// supposed to be serializable. + fn encode_value(&self, value: &V) -> Vec; + + /// Tries to deserialize a value from a bytes slice, and returns a + /// [`Result`] with either the deserialized value or an error. + fn try_decode_value(&self, bytes: &[u8]) -> Result; + + /// Deserializes a value from a bytes slice. + /// + /// # Panics + /// Panics if the call to [`StateValueCodec::try_decode_value`] fails. Use + /// [`StateValueCodec::try_decode_value`] if you need to gracefully handle + /// errors. + fn decode_value(&self, bytes: &[u8]) -> V { + self.try_decode_value(bytes) + .map_err(|err| { + format!( + "Failed to decode value 0x{}, error: {:?}", + hex::encode(bytes), + err + ) + }) + .unwrap() + } +} + +/// A market trait for types that implement both [`StateKeyCodec`] and +/// [`StateValueCodec`]. +pub trait StateCodec: StateKeyCodec + StateValueCodec {} + +impl StateCodec for C where C: StateKeyCodec + StateValueCodec {} + +/// A [`StateCodec`] that uses [`borsh`] for all keys and values. +#[derive(Debug, Default, PartialEq, Eq, Clone, borsh::BorshDeserialize, borsh::BorshSerialize)] +pub struct BorshCodec; + +impl StateKeyCodec for BorshCodec +where + K: borsh::BorshSerialize + borsh::BorshDeserialize, +{ + type KeyError = std::io::Error; + + fn encode_key(&self, key: &K) -> Vec { + key.try_to_vec().expect("Failed to serialize key") + } + + fn try_decode_key(&self, bytes: &[u8]) -> Result { + K::try_from_slice(bytes) + } +} + +impl StateValueCodec for BorshCodec +where + V: borsh::BorshSerialize + borsh::BorshDeserialize, +{ + type ValueError = std::io::Error; + + fn encode_value(&self, value: &V) -> Vec { + value.try_to_vec().expect("Failed to serialize value") + } + + fn try_decode_value(&self, bytes: &[u8]) -> Result { + V::try_from_slice(bytes) + } +} + +/// A [`StateCodec`] that uses two different codecs under the hood, one for keys +/// and one for values. +pub struct PairOfCodecs { + pub key_codec: KC, + pub value_codec: VC, +} + +impl StateKeyCodec for PairOfCodecs +where + KC: StateKeyCodec, +{ + type KeyError = KC::KeyError; + + fn decode_key(&self, bytes: &[u8]) -> K { + self.key_codec.decode_key(bytes) + } + + fn try_decode_key(&self, bytes: &[u8]) -> Result { + self.key_codec.try_decode_key(bytes) + } + + fn encode_key(&self, key: &K) -> Vec { + self.key_codec.encode_key(key) + } +} + +impl StateValueCodec for PairOfCodecs +where + VC: StateValueCodec, +{ + type ValueError = VC::ValueError; + + fn decode_value(&self, bytes: &[u8]) -> V { + self.value_codec.decode_value(bytes) + } + + fn try_decode_value(&self, bytes: &[u8]) -> Result { + self.value_codec.try_decode_value(bytes) + } + + fn encode_value(&self, value: &V) -> Vec { + self.value_codec.encode_value(value) + } +} diff --git a/module-system/sov-state/src/lib.rs b/module-system/sov-state/src/lib.rs index e70c3aeaf..360f6167d 100644 --- a/module-system/sov-state/src/lib.rs +++ b/module-system/sov-state/src/lib.rs @@ -1,3 +1,4 @@ +pub mod codec; mod internal_cache; mod map; @@ -15,8 +16,6 @@ mod utils; mod value; mod witness; -pub use value::SingletonKey; - mod zk_storage; pub use zk_storage::ZkStorage; diff --git a/module-system/sov-state/src/map.rs b/module-system/sov-state/src/map.rs index be677d0bd..2fae63ac2 100644 --- a/module-system/sov-state/src/map.rs +++ b/module-system/sov-state/src/map.rs @@ -1,16 +1,25 @@ use std::marker::PhantomData; -use borsh::{BorshDeserialize, BorshSerialize}; use thiserror::Error; +use crate::codec::{BorshCodec, StateCodec}; use crate::storage::StorageKey; use crate::{Prefix, Storage, WorkingSet}; /// A container that maps keys to values. - +/// +/// # Type parameters +/// [`StateMap`] is generic over: +/// - a key type (`K`); +/// - a value type (`V`); +/// - a [`StateCodec`] (`C`). #[derive(borsh::BorshDeserialize, borsh::BorshSerialize, Debug, PartialEq, Clone)] -pub struct StateMap { +pub struct StateMap +where + C: StateCodec, +{ _phantom: (PhantomData, PhantomData), + codec: C, prefix: Prefix, } @@ -21,22 +30,51 @@ pub enum Error { MissingValue(Prefix, StorageKey), } -impl StateMap { +impl StateMap +where + BorshCodec: StateCodec, +{ + /// Creates a new [`StateMap`] with the given prefix and the default + /// [`StateCodec`] (i.e. [`BorshCodec`]). pub fn new(prefix: Prefix) -> Self { Self { _phantom: (PhantomData, PhantomData), + codec: BorshCodec, + prefix, + } + } +} + +impl StateMap +where + C: StateCodec, +{ + /// Creates a new [`StateMap`] with the given prefix and codec. + /// + /// Note that `codec` must implement both [`StateKeyCodec`] and + /// [`StateValueCodec`] and there's no way (yet?) to use different codecs + /// for keys and values. + pub fn with_codec(prefix: Prefix, codec: C) -> Self { + Self { + _phantom: (PhantomData, PhantomData), + codec, prefix, } } + /// Returns the prefix used when this [`StateValue`] was created. + pub fn prefix(&self) -> &Prefix { + &self.prefix + } + /// Inserts a key-value pair into the map. pub fn set(&self, key: &K, value: &V, working_set: &mut WorkingSet) { - working_set.set_value(self.prefix(), key, value) + working_set.set_value(self.prefix(), &self.codec, key, value) } /// Returns the value corresponding to the key or None if key is absent in the StateMap. pub fn get(&self, key: &K, working_set: &mut WorkingSet) -> Option { - working_set.get_value(self.prefix(), key) + working_set.get_value(self.prefix(), &self.codec, key) } /// Returns the value corresponding to the key or Error if key is absent in the StateMap. @@ -46,13 +84,16 @@ impl StateMap { working_set: &mut WorkingSet, ) -> Result { self.get(key, working_set).ok_or_else(|| { - Error::MissingValue(self.prefix().clone(), StorageKey::new(self.prefix(), key)) + Error::MissingValue( + self.prefix().clone(), + StorageKey::new(self.prefix(), key, &self.codec), + ) }) } /// Removes a key from the StateMap, returning the corresponding value (or None if the key is absent). pub fn remove(&self, key: &K, working_set: &mut WorkingSet) -> Option { - working_set.remove_value(self.prefix(), key) + working_set.remove_value(self.prefix(), &self.codec, key) } /// Removes a key from the StateMap, returning the corresponding value (or Error if the key is absent). @@ -62,16 +103,15 @@ impl StateMap { working_set: &mut WorkingSet, ) -> Result { self.remove(key, working_set).ok_or_else(|| { - Error::MissingValue(self.prefix().clone(), StorageKey::new(self.prefix(), key)) + Error::MissingValue( + self.prefix().clone(), + StorageKey::new(self.prefix(), key, &self.codec), + ) }) } /// Deletes a key from the StateMap. pub fn delete(&self, key: &K, working_set: &mut WorkingSet) { - working_set.delete_value(self.prefix(), key); - } - - pub fn prefix(&self) -> &Prefix { - &self.prefix + working_set.delete_value(self.prefix(), &self.codec, key); } } diff --git a/module-system/sov-state/src/prover_storage.rs b/module-system/sov-state/src/prover_storage.rs index 6c4a039d0..da471bdde 100644 --- a/module-system/sov-state/src/prover_storage.rs +++ b/module-system/sov-state/src/prover_storage.rs @@ -7,6 +7,7 @@ use jmt::storage::TreeWriter; use jmt::{JellyfishMerkleTree, KeyHash, RootHash, Version}; use sov_db::state_db::StateDB; +use crate::codec::BorshCodec; use crate::config::Config; use crate::internal_cache::OrderedReadsAndWrites; use crate::storage::{NativeStorage, StorageKey, StorageProof, StorageValue}; @@ -179,7 +180,10 @@ impl NativeStorage for ProverStorage { self.db.get_next_version() - 1, ) .unwrap(); - (val_opt.as_ref().map(StorageValue::new), proof) + ( + val_opt.as_ref().map(|v| StorageValue::new(v, &BorshCodec)), + proof, + ) } } diff --git a/module-system/sov-state/src/scratchpad.rs b/module-system/sov-state/src/scratchpad.rs index 1773e3a16..271c39f44 100644 --- a/module-system/sov-state/src/scratchpad.rs +++ b/module-system/sov-state/src/scratchpad.rs @@ -1,10 +1,10 @@ use std::collections::HashMap; use std::fmt::Debug; -use borsh::{BorshDeserialize, BorshSerialize}; use sov_first_read_last_write_cache::{CacheKey, CacheValue}; use sov_rollup_interface::stf::Event; +use crate::codec::{StateCodec, StateKeyCodec, StateValueCodec}; use crate::internal_cache::{OrderedReadsAndWrites, StorageInternalCache}; use crate::storage::{StorageKey, StorageValue}; use crate::{Prefix, Storage}; @@ -135,50 +135,63 @@ impl WorkingSet { } impl WorkingSet { - pub(crate) fn set_value( + pub(crate) fn set_value( &mut self, prefix: &Prefix, + codec: &C, storage_key: &K, value: &V, - ) { - let storage_key = StorageKey::new(prefix, storage_key); - let storage_value = StorageValue::new(value); + ) where + C: StateCodec, + { + let storage_key = StorageKey::new(prefix, storage_key, codec); + let storage_value = StorageValue::new(value, codec); self.set(storage_key, storage_value); } - pub(crate) fn get_value( + pub(crate) fn get_value( &mut self, prefix: &Prefix, + codec: &C, storage_key: &K, - ) -> Option { - let storage_key = StorageKey::new(prefix, storage_key); - self.get_decoded(storage_key) + ) -> Option + where + C: StateCodec, + { + let storage_key = StorageKey::new(prefix, storage_key, codec); + self.get_decoded(codec, storage_key) } - pub(crate) fn remove_value( + pub(crate) fn remove_value( &mut self, prefix: &Prefix, + codec: &C, storage_key: &K, - ) -> Option { - let storage_key = StorageKey::new(prefix, storage_key); - let storage_value = self.get_decoded(storage_key.clone())?; + ) -> Option + where + C: StateCodec, + { + let storage_key = StorageKey::new(prefix, storage_key, codec); + let storage_value = self.get_decoded(codec, storage_key.clone())?; self.delete(storage_key); Some(storage_value) } - pub(crate) fn delete_value(&mut self, prefix: &Prefix, storage_key: &K) { - let storage_key = StorageKey::new(prefix, storage_key); + pub(crate) fn delete_value(&mut self, prefix: &Prefix, codec: &C, storage_key: &K) + where + C: StateKeyCodec, + { + let storage_key = StorageKey::new(prefix, storage_key, codec); self.delete(storage_key); } - fn get_decoded(&mut self, storage_key: StorageKey) -> Option { + fn get_decoded(&mut self, codec: &C, storage_key: StorageKey) -> Option + where + C: StateValueCodec, + { let storage_value = self.get(storage_key)?; - // It is ok to panic here. Deserialization problem means that something is terribly wrong. - Some( - V::deserialize_reader(&mut storage_value.value()) - .unwrap_or_else(|e| panic!("Unable to deserialize storage value {e:?}")), - ) + Some(codec.decode_value(storage_value.value())) } } diff --git a/module-system/sov-state/src/storage.rs b/module-system/sov-state/src/storage.rs index 58e0617fc..ed3080a0f 100644 --- a/module-system/sov-state/src/storage.rs +++ b/module-system/sov-state/src/storage.rs @@ -7,6 +7,7 @@ use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; use sov_first_read_last_write_cache::{CacheKey, CacheValue}; +use crate::codec::{StateKeyCodec, StateValueCodec}; use crate::internal_cache::OrderedReadsAndWrites; use crate::utils::AlignedVec; use crate::witness::Witness; @@ -48,8 +49,11 @@ impl Display for StorageKey { impl StorageKey { /// Creates a new StorageKey that combines a prefix and a key. - pub fn new(prefix: &Prefix, key: &K) -> Self { - let encoded_key = key.try_to_vec().unwrap(); + pub fn new(prefix: &Prefix, key: &K, codec: &C) -> Self + where + C: StateKeyCodec, + { + let encoded_key = codec.encode_key(key); let encoded_key = AlignedVec::new(encoded_key); let full_key = Vec::::with_capacity(prefix.len() + encoded_key.len()); @@ -88,9 +92,12 @@ impl From> for StorageValue { } impl StorageValue { - /// Create a new storage value by serializing the input - pub fn new(value: &impl BorshSerialize) -> Self { - let encoded_value = value.try_to_vec().unwrap(); + /// Create a new storage value by serializing the input with the given codec. + pub fn new(value: &V, codec: &C) -> Self + where + C: StateValueCodec, + { + let encoded_value = codec.encode_value(value); Self { value: Arc::new(encoded_value), } diff --git a/module-system/sov-state/src/value.rs b/module-system/sov-state/src/value.rs index 6f7e1b065..c525c9c6e 100644 --- a/module-system/sov-state/src/value.rs +++ b/module-system/sov-state/src/value.rs @@ -1,29 +1,16 @@ -use std::io::Write; use std::marker::PhantomData; use borsh::{BorshDeserialize, BorshSerialize}; use thiserror::Error; +use crate::codec::{BorshCodec, StateKeyCodec, StateValueCodec}; use crate::{Prefix, Storage, WorkingSet}; -// SingletonKey is very similar to the unit type `()` i.e. it has only one value. -#[derive(Debug, BorshDeserialize)] -pub struct SingletonKey; - -impl BorshSerialize for SingletonKey { - fn serialize(&self, _writer: &mut W) -> std::io::Result<()> { - Ok(()) - } - - fn try_to_vec(&self) -> std::io::Result> { - Ok(vec![]) - } -} - /// Container for a single value. #[derive(Debug, PartialEq, Eq, Clone, BorshDeserialize, BorshSerialize)] -pub struct StateValue { +pub struct StateValue { _phantom: PhantomData, + codec: C, prefix: Prefix, } @@ -34,22 +21,51 @@ pub enum Error { MissingValue(Prefix), } -impl StateValue { +impl StateValue +where + BorshCodec: StateValueCodec, +{ + /// Crates a new [`StateValue`] with the given prefix and the default + /// [`StateCodec`] (i.e. [`BorshCodec`]). pub fn new(prefix: Prefix) -> Self { Self { _phantom: PhantomData, + codec: BorshCodec, + prefix, + } + } +} + +impl StateValue +where + C: StateValueCodec, +{ + /// Creates a new [`StateValue`] with the given prefix and codec. + pub fn with_codec(prefix: Prefix, codec: C) -> Self { + Self { + _phantom: PhantomData, + codec, prefix, } } + /// Returns the prefix used when this [`StateValue`] was created. + pub fn prefix(&self) -> &Prefix { + &self.prefix + } + + fn internal_codec(&self) -> SingletonCodec { + SingletonCodec::new(&self.codec) + } + /// Sets a value in the StateValue. pub fn set(&self, value: &V, working_set: &mut WorkingSet) { - working_set.set_value(self.prefix(), &SingletonKey, value) + working_set.set_value(self.prefix(), &self.internal_codec(), &SingletonKey, value) } /// Gets a value from the StateValue or None if the value is absent. pub fn get(&self, working_set: &mut WorkingSet) -> Option { - working_set.get_value(self.prefix(), &SingletonKey) + working_set.get_value(self.prefix(), &self.internal_codec(), &SingletonKey) } /// Gets a value from the StateValue or Error if the value is absent. @@ -60,7 +76,7 @@ impl StateValue { /// Removes a value from the StateValue, returning the value (or None if the key is absent). pub fn remove(&self, working_set: &mut WorkingSet) -> Option { - working_set.remove_value(self.prefix(), &SingletonKey) + working_set.remove_value(self.prefix(), &self.internal_codec(), &SingletonKey) } /// Removes a value and from the StateValue, returning the value (or Error if the key is absent). @@ -71,10 +87,55 @@ impl StateValue { /// Deletes a value from the StateValue. pub fn delete(&self, working_set: &mut WorkingSet) { - working_set.delete_value(self.prefix(), &SingletonKey); + working_set.delete_value(self.prefix(), &self.internal_codec(), &SingletonKey); } +} - pub fn prefix(&self) -> &Prefix { - &self.prefix +// SingletonKey is very similar to the unit type `()` i.e. it has only one value. +#[derive(Debug)] +struct SingletonKey; + +/// Skips (de)serialization of keys and delegates values to another codec. +struct SingletonCodec<'a, VC> { + value_codec: &'a VC, +} + +impl<'a, VC> SingletonCodec<'a, VC> { + pub fn new(value_codec: &'a VC) -> Self { + Self { value_codec } + } +} + +impl<'a, VC> StateKeyCodec for SingletonCodec<'a, VC> { + type KeyError = std::io::Error; + + fn encode_key(&self, _: &SingletonKey) -> Vec { + vec![] + } + + fn try_decode_key(&self, bytes: &[u8]) -> Result { + if bytes.is_empty() { + Ok(SingletonKey) + } else { + Err(std::io::Error::new( + std::io::ErrorKind::InvalidData, + "SingletonKey must be empty", + )) + } + } +} + +impl<'a, V, VC> StateValueCodec for SingletonCodec<'a, VC> +where + VC: StateValueCodec, +{ + type ValueError = VC::ValueError; + + fn encode_value(&self, value: &V) -> Vec { + self.value_codec.encode_value(value) + } + + fn try_decode_value(&self, bytes: &[u8]) -> Result { + self.value_codec.try_decode_value(bytes) } }