From 9cb902b6d345c9f09592a0eae8d3aaf9316c069a Mon Sep 17 00:00:00 2001 From: Preston Evans Date: Thu, 12 Oct 2023 13:16:05 -0500 Subject: [PATCH 1/5] Add constants to manifest --- constants.json | 28 ++++- .../sov-modules-api/src/reexport_macros.rs | 3 + module-system/sov-modules-macros/src/lib.rs | 11 ++ .../sov-modules-macros/src/make_constants.rs | 12 ++ .../sov-modules-macros/src/manifest.rs | 109 +++++++++++++++++- .../sov-modules-macros/tests/all_tests.rs | 13 +++ .../sov-modules-macros/tests/constants.json | 78 +++++++++++++ .../tests/constants/create_constant.rs | 23 ++++ 8 files changed, 269 insertions(+), 8 deletions(-) create mode 100644 module-system/sov-modules-macros/src/make_constants.rs create mode 100644 module-system/sov-modules-macros/tests/constants.json create mode 100644 module-system/sov-modules-macros/tests/constants/create_constant.rs diff --git a/constants.json b/constants.json index 9beb309f4..ea8daaa60 100644 --- a/constants.json +++ b/constants.json @@ -2,11 +2,29 @@ "comment": "Sovereign SDK constants", "gas": { "Bank": { - "create_token": [4, 4], - "transfer": [5, 5], - "burn": [2, 2], - "mint": [2, 2], - "freeze": [1, 1] + "create_token": [ + 4, + 4 + ], + "transfer": [ + 5, + 5 + ], + "burn": [ + 2, + 2 + ], + "mint": [ + 2, + 2 + ], + "freeze": [ + 1, + 1 + ] } + }, + "constants": { + "TEST_U32": 42 } } diff --git a/module-system/sov-modules-api/src/reexport_macros.rs b/module-system/sov-modules-api/src/reexport_macros.rs index ce3f4961c..eeb74edcc 100644 --- a/module-system/sov-modules-api/src/reexport_macros.rs +++ b/module-system/sov-modules-api/src/reexport_macros.rs @@ -178,4 +178,7 @@ pub mod macros { /// trait for the Runtime because the stdlib implementation of the default trait imposes the generic /// arguments to have the Default trait, which is not needed in our case. pub use sov_modules_macros::DefaultRuntime; + + /// Sets the value of a constant at compile time by reading from the Manifest file. + pub use sov_modules_macros::config_constant; } diff --git a/module-system/sov-modules-macros/src/lib.rs b/module-system/sov-modules-macros/src/lib.rs index c5bd5d1ca..6fea48fa3 100644 --- a/module-system/sov-modules-macros/src/lib.rs +++ b/module-system/sov-modules-macros/src/lib.rs @@ -14,6 +14,7 @@ mod cli_parser; mod common; mod default_runtime; mod dispatch; +mod make_constants; mod manifest; mod module_call_json_schema; mod module_info; @@ -28,6 +29,7 @@ use default_runtime::DefaultRuntimeMacro; use dispatch::dispatch_call::DispatchCallMacro; use dispatch::genesis::GenesisMacro; use dispatch::message_codec::MessageCodec; +use make_constants::make_const; use module_call_json_schema::derive_module_call_json_schema; use new_types::address_type_helper; use offchain::offchain_generator; @@ -82,6 +84,15 @@ pub fn codec(input: TokenStream) -> TokenStream { handle_macro_error(codec_macro.derive_message_codec(input)) } +/// Specifies that a constant should be configured from the manifest file. +#[proc_macro_attribute] +pub fn config_constant(_attr: TokenStream, item: TokenStream) -> TokenStream { + let input = parse_macro_input!(item as syn::ItemConst); + handle_macro_error( + make_const(&input.ident, &input.ty, input.vis, &input.attrs).map(|ok| ok.into()), + ) +} + /// Derives a [`jsonrpsee`] implementation for the underlying type. Any code relying on this macro /// must take jsonrpsee as a dependency with at least the following features enabled: `["macros", "client-core", "server"]`. /// diff --git a/module-system/sov-modules-macros/src/make_constants.rs b/module-system/sov-modules-macros/src/make_constants.rs new file mode 100644 index 000000000..b10f5b90c --- /dev/null +++ b/module-system/sov-modules-macros/src/make_constants.rs @@ -0,0 +1,12 @@ +use syn::{Ident, Type}; + +use crate::manifest::Manifest; + +pub(crate) fn make_const( + field_ident: &Ident, + ty: &Type, + vis: syn::Visibility, + attrs: &[syn::Attribute], +) -> Result { + Manifest::read_constants(field_ident)?.parse_constant(ty, field_ident, vis, attrs) +} diff --git a/module-system/sov-modules-macros/src/manifest.rs b/module-system/sov-modules-macros/src/manifest.rs index 5f778620f..46aefa8aa 100644 --- a/module-system/sov-modules-macros/src/manifest.rs +++ b/module-system/sov-modules-macros/src/manifest.rs @@ -2,6 +2,7 @@ use std::path::{Path, PathBuf}; use std::{env, fmt, fs, ops}; use proc_macro2::{Ident, TokenStream}; +use quote::format_ident; use serde_json::Value; use syn::{PathArguments, Type, TypePath}; @@ -48,12 +49,16 @@ impl<'a> Manifest<'a> { /// The `parent` is used to report the errors to the correct span location. pub fn read_constants(parent: &'a Ident) -> Result { let manifest = "constants.json"; - let initial_path = match env::var("CONSTANTS_MANIFEST") { + let initial_path = match env::var("CONSTANTS_MANIFEST").and_then(|v| if v.is_empty() { + Err(env::VarError::NotPresent) + } else { + Ok(v) + }) { Ok(p) => PathBuf::from(&p).canonicalize().map_err(|e| { Self::err( &p, parent, - format!("failed access base dir for sovereign manifest file `{p}`: {e}"), + format!("failed to canonicalize path for sovereign manifest file from env var `{p}`: {e}"), ) }), Err(_) => { @@ -168,7 +173,7 @@ impl<'a> Manifest<'a> { Self::err( &self.path, field, - format!("failed to parse key attribyte `{}`: {}", k, e), + format!("failed to parse key attribute `{}`: {}", k, e), ) })?; @@ -236,6 +241,104 @@ impl<'a> Manifest<'a> { }) } + pub fn parse_constant( + &self, + ty: &Type, + field: &Ident, + vis: syn::Visibility, + attrs: &[syn::Attribute], + ) -> Result { + let map = self + .value + .as_object() + .ok_or_else(|| Self::err(&self.path, field, "manifest is not an object"))?; + + let root = map + .get("constants") + .ok_or_else(|| { + Self::err( + &self.path, + field, + "manifest does not contain a `constants` attribute", + ) + })? + .as_object() + .ok_or_else(|| { + Self::err( + &self.path, + field, + format!("`constants` attribute of `{}` is not an object", field), + ) + })?; + let value = root.get(&field.to_string()).ok_or_else(|| { + Self::err( + &self.path, + field, + format!("manifest does not contain a `{}` attribute", field), + ) + })?; + let value = self.value_to_tokens(field, value, ty)?; + let output = quote::quote! { + #(#attrs)* + #vis const #field: #ty = #value; + }; + Ok(output) + } + + fn value_to_tokens( + &self, + field: &Ident, + value: &serde_json::Value, + ty: &Type, + ) -> Result { + match value { + Value::Null => { + return Err(Self::err( + &self.path, + field, + format!("`{}` is `null`", field), + )) + } + Value::Bool(b) => Ok(quote::quote!(#b)), + Value::Number(n) => { + if n.is_u64() { + let n = n.as_u64().unwrap(); + Ok(quote::quote!(#n as #ty)) + } else if n.is_i64() { + let n = n.as_i64().unwrap(); + Ok(quote::quote!(#n as #ty)) + } else { + Err(Self::err(&self.path, field, format!("All numeric values must be representable as 64 bit integers during parsing."))) + } + } + Value::String(s) => Ok(quote::quote!(#s)), + Value::Array(arr) => { + let mut values = Vec::with_capacity(arr.len()); + let ty = if let Type::Array(ty) = ty { + &ty.elem + } else { + return Err(Self::err( + &self.path, + field, + format!( + "Found value of type {:?} while parsing `{}` but expected an array type ", + ty, field + ), + )); + }; + for (idx, value) in arr.iter().enumerate() { + values.push(self.value_to_tokens( + &format_ident!("{field}_{idx}"), + value, + ty, + )?); + } + Ok(quote::quote!([#(#values,)*])) + } + Value::Object(_) => todo!(), + } + } + fn err(path: P, ident: &syn::Ident, msg: T) -> syn::Error where P: AsRef, diff --git a/module-system/sov-modules-macros/tests/all_tests.rs b/module-system/sov-modules-macros/tests/all_tests.rs index 4111c6f3d..a716cf8be 100644 --- a/module-system/sov-modules-macros/tests/all_tests.rs +++ b/module-system/sov-modules-macros/tests/all_tests.rs @@ -1,3 +1,5 @@ +use std::path::PathBuf; + #[test] fn module_info_tests() { let t = trybuild::TestCases::new(); @@ -46,3 +48,14 @@ fn cli_wallet_arg_tests() { t.pass("tests/cli_wallet_arg/derive_enum_unnamed_fields.rs"); t.pass("tests/cli_wallet_arg/derive_wallet.rs"); } + +#[test] +fn constants_from_manifests_test() { + let t: trybuild::TestCases = trybuild::TestCases::new(); + let manifest_dir = std::env::var_os("CARGO_MANIFEST_DIR").unwrap(); + std::env::set_var( + "CONSTANTS_MANIFEST", + PathBuf::from(manifest_dir).join("tests"), + ); + t.pass("tests/constants/create_constant.rs"); +} diff --git a/module-system/sov-modules-macros/tests/constants.json b/module-system/sov-modules-macros/tests/constants.json new file mode 100644 index 000000000..0f85bcff7 --- /dev/null +++ b/module-system/sov-modules-macros/tests/constants.json @@ -0,0 +1,78 @@ +{ + "comment": "Sovereign SDK constants", + "constants": { + "TEST_U32": 42, + "TEST_BOOL": true, + "TEST_STRING": "Some Other String", + "TEST_NESTED_ARRAY": [ + [ + 7, + 7, + 7 + ], + [ + 7, + 7, + 7 + ] + ], + "TEST_ARRAY_OF_U8": [ + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11, + 11 + ] + }, + "gas": { + "Bank": { + "create_token": [ + 4, + 4 + ], + "transfer": [ + 5, + 5 + ], + "burn": [ + 2, + 2 + ], + "mint": [ + 2, + 2 + ], + "freeze": [ + 1, + 1 + ] + } + } +} diff --git a/module-system/sov-modules-macros/tests/constants/create_constant.rs b/module-system/sov-modules-macros/tests/constants/create_constant.rs new file mode 100644 index 000000000..f46240b1d --- /dev/null +++ b/module-system/sov-modules-macros/tests/constants/create_constant.rs @@ -0,0 +1,23 @@ +use sov_modules_api::macros::config_constant; +#[config_constant] +pub const TEST_U32: u32 = 0; + +#[config_constant] +pub const TEST_ARRAY_OF_U8: [u8; 32] = [0; 32]; + +#[config_constant] +pub const TEST_NESTED_ARRAY: [[u8; 3]; 2] = [[0; 3]; 2]; + +#[config_constant] +pub const TEST_BOOL: bool = false; + +#[config_constant] +pub const TEST_STRING: &str = "Some String"; + +fn main() { + assert_eq!(TEST_U32, 42); + assert_eq!(TEST_ARRAY_OF_U8, [11; 32]); + assert_eq!(TEST_NESTED_ARRAY, [[7; 3]; 2]); + assert_eq!(TEST_BOOL, true); + assert_eq!(TEST_STRING, "Some Other String"); +} From 4a0377909521f71873e8f8350fa0650c36bedab5 Mon Sep 17 00:00:00 2001 From: Preston Evans Date: Thu, 12 Oct 2023 13:28:08 -0500 Subject: [PATCH 2/5] fmt --- module-system/sov-modules-api/src/reexport_macros.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/module-system/sov-modules-api/src/reexport_macros.rs b/module-system/sov-modules-api/src/reexport_macros.rs index eeb74edcc..a29d62677 100644 --- a/module-system/sov-modules-api/src/reexport_macros.rs +++ b/module-system/sov-modules-api/src/reexport_macros.rs @@ -80,6 +80,8 @@ pub use sov_modules_macros::ModuleInfo; /// Procedural macros to assist with creating new modules. #[cfg(feature = "macros")] pub mod macros { + /// Sets the value of a constant at compile time by reading from the Manifest file. + pub use sov_modules_macros::config_constant; /// The macro exposes RPC endpoints from all modules in the runtime. /// It gets storage from the Context generic /// and utilizes output of [`#[rpc_gen]`] macro to generate RPC methods. @@ -178,7 +180,4 @@ pub mod macros { /// trait for the Runtime because the stdlib implementation of the default trait imposes the generic /// arguments to have the Default trait, which is not needed in our case. pub use sov_modules_macros::DefaultRuntime; - - /// Sets the value of a constant at compile time by reading from the Manifest file. - pub use sov_modules_macros::config_constant; } From cdd89594956d93680ad2e24f74418f08109b7f65 Mon Sep 17 00:00:00 2001 From: Preston Evans Date: Thu, 12 Oct 2023 14:21:24 -0500 Subject: [PATCH 3/5] fix nit --- .../sov-modules-macros/src/manifest.rs | 98 ++++++++----------- 1 file changed, 40 insertions(+), 58 deletions(-) diff --git a/module-system/sov-modules-macros/src/manifest.rs b/module-system/sov-modules-macros/src/manifest.rs index 46aefa8aa..084eece88 100644 --- a/module-system/sov-modules-macros/src/manifest.rs +++ b/module-system/sov-modules-macros/src/manifest.rs @@ -3,7 +3,7 @@ use std::{env, fmt, fs, ops}; use proc_macro2::{Ident, TokenStream}; use quote::format_ident; -use serde_json::Value; +use serde_json::{Map, Value}; use syn::{PathArguments, Type, TypePath}; #[derive(Debug, Clone)] @@ -49,11 +49,14 @@ impl<'a> Manifest<'a> { /// The `parent` is used to report the errors to the correct span location. pub fn read_constants(parent: &'a Ident) -> Result { let manifest = "constants.json"; - let initial_path = match env::var("CONSTANTS_MANIFEST").and_then(|v| if v.is_empty() { - Err(env::VarError::NotPresent) - } else { - Ok(v) - }) { + let initial_path = match env::var("CONSTANTS_MANIFEST"){ + Ok(p) if p.is_empty() => { + Err(Self::err( + &p, + parent, + format!("failed to read target path for sovereign manifest file: env var `CONSTANTS_MANIFEST` was set to the empty string"), + )) + }, Ok(p) => PathBuf::from(&p).canonicalize().map_err(|e| { Self::err( &p, @@ -117,6 +120,29 @@ impl<'a> Manifest<'a> { Self::read_str(manifest, path, parent) } + /// Gets the requested object from the manifest by key + fn get_object(&self, field: &Ident, key: &str) -> Result<&Map, syn::Error> { + self.value + .as_object() + .ok_or_else(|| Self::err(&self.path, field, "manifest is not an object"))? + .get(key) + .ok_or_else(|| { + Self::err( + &self.path, + field, + format!("manifest does not contain a `{key}` attribute"), + ) + })? + .as_object() + .ok_or_else(|| { + Self::err( + &self.path, + field, + format!("`{key}` attribute of `{field}` is not an object"), + ) + }) + } + /// Parses a gas config constant from the manifest file. Returns a `TokenStream` with the /// following structure: /// @@ -132,28 +158,7 @@ impl<'a> Manifest<'a> { /// The `gas` field resolution will first attempt to query `gas.parent`, and then fallback to /// `gas`. They must be objects with arrays of integers as fields. pub fn parse_gas_config(&self, ty: &Type, field: &Ident) -> Result { - let map = self - .value - .as_object() - .ok_or_else(|| Self::err(&self.path, field, "manifest is not an object"))?; - - let root = map - .get("gas") - .ok_or_else(|| { - Self::err( - &self.path, - field, - "manifest does not contain a `gas` attribute", - ) - })? - .as_object() - .ok_or_else(|| { - Self::err( - &self.path, - field, - format!("`gas` attribute of `{}` is not an object", field), - ) - })?; + let root = self.get_object(field, "gas")?; let root = match root.get(&self.parent.to_string()) { Some(Value::Object(m)) => m, @@ -248,28 +253,7 @@ impl<'a> Manifest<'a> { vis: syn::Visibility, attrs: &[syn::Attribute], ) -> Result { - let map = self - .value - .as_object() - .ok_or_else(|| Self::err(&self.path, field, "manifest is not an object"))?; - - let root = map - .get("constants") - .ok_or_else(|| { - Self::err( - &self.path, - field, - "manifest does not contain a `constants` attribute", - ) - })? - .as_object() - .ok_or_else(|| { - Self::err( - &self.path, - field, - format!("`constants` attribute of `{}` is not an object", field), - ) - })?; + let root = self.get_object(field, "constants")?; let value = root.get(&field.to_string()).ok_or_else(|| { Self::err( &self.path, @@ -292,13 +276,11 @@ impl<'a> Manifest<'a> { ty: &Type, ) -> Result { match value { - Value::Null => { - return Err(Self::err( - &self.path, - field, - format!("`{}` is `null`", field), - )) - } + Value::Null => Err(Self::err( + &self.path, + field, + format!("`{}` is `null`", field), + )), Value::Bool(b) => Ok(quote::quote!(#b)), Value::Number(n) => { if n.is_u64() { @@ -308,7 +290,7 @@ impl<'a> Manifest<'a> { let n = n.as_i64().unwrap(); Ok(quote::quote!(#n as #ty)) } else { - Err(Self::err(&self.path, field, format!("All numeric values must be representable as 64 bit integers during parsing."))) + Err(Self::err(&self.path, field, "All numeric values must be representable as 64 bit integers during parsing.".to_string())) } } Value::String(s) => Ok(quote::quote!(#s)), From 583484da99f5d7bac6156cfe5eeb4a23bb89066c Mon Sep 17 00:00:00 2001 From: Preston Evans Date: Thu, 12 Oct 2023 14:22:23 -0500 Subject: [PATCH 4/5] clippy --- module-system/sov-modules-macros/src/manifest.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module-system/sov-modules-macros/src/manifest.rs b/module-system/sov-modules-macros/src/manifest.rs index 084eece88..60f25edc4 100644 --- a/module-system/sov-modules-macros/src/manifest.rs +++ b/module-system/sov-modules-macros/src/manifest.rs @@ -54,7 +54,7 @@ impl<'a> Manifest<'a> { Err(Self::err( &p, parent, - format!("failed to read target path for sovereign manifest file: env var `CONSTANTS_MANIFEST` was set to the empty string"), + "failed to read target path for sovereign manifest file: env var `CONSTANTS_MANIFEST` was set to the empty string".to_string(), )) }, Ok(p) => PathBuf::from(&p).canonicalize().map_err(|e| { From 2e90d22e05231a3b5b120201f806d7eb9e933aab Mon Sep 17 00:00:00 2001 From: Preston Evans Date: Thu, 12 Oct 2023 14:50:41 -0500 Subject: [PATCH 5/5] Add custom parsing for consts --- module-system/sov-modules-macros/src/lib.rs | 6 ++-- .../sov-modules-macros/src/make_constants.rs | 28 ++++++++++++++++++- .../tests/constants/create_constant.rs | 12 ++++---- 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/module-system/sov-modules-macros/src/lib.rs b/module-system/sov-modules-macros/src/lib.rs index 6fea48fa3..136c96e9f 100644 --- a/module-system/sov-modules-macros/src/lib.rs +++ b/module-system/sov-modules-macros/src/lib.rs @@ -29,7 +29,7 @@ use default_runtime::DefaultRuntimeMacro; use dispatch::dispatch_call::DispatchCallMacro; use dispatch::genesis::GenesisMacro; use dispatch::message_codec::MessageCodec; -use make_constants::make_const; +use make_constants::{make_const, PartialItemConst}; use module_call_json_schema::derive_module_call_json_schema; use new_types::address_type_helper; use offchain::offchain_generator; @@ -84,10 +84,10 @@ pub fn codec(input: TokenStream) -> TokenStream { handle_macro_error(codec_macro.derive_message_codec(input)) } -/// Specifies that a constant should be configured from the manifest file. +/// Sets a constant from the manifest file instead of hard-coding it inline. #[proc_macro_attribute] pub fn config_constant(_attr: TokenStream, item: TokenStream) -> TokenStream { - let input = parse_macro_input!(item as syn::ItemConst); + let input = parse_macro_input!(item as PartialItemConst); handle_macro_error( make_const(&input.ident, &input.ty, input.vis, &input.attrs).map(|ok| ok.into()), ) diff --git a/module-system/sov-modules-macros/src/make_constants.rs b/module-system/sov-modules-macros/src/make_constants.rs index b10f5b90c..e9809a9d4 100644 --- a/module-system/sov-modules-macros/src/make_constants.rs +++ b/module-system/sov-modules-macros/src/make_constants.rs @@ -1,7 +1,33 @@ -use syn::{Ident, Type}; +use syn::parse::Parse; +use syn::{Attribute, Ident, Token, Type, Visibility}; use crate::manifest::Manifest; +/// A partial const declaration: `const MAX: u16;`. +pub struct PartialItemConst { + pub attrs: Vec, + pub vis: Visibility, + pub const_token: Token![const], + pub ident: Ident, + pub colon_token: Token![:], + pub ty: Type, + pub semi_token: Token![;], +} + +impl Parse for PartialItemConst { + fn parse(input: syn::parse::ParseStream) -> syn::Result { + Ok(Self { + attrs: Attribute::parse_outer(input)?, + vis: input.parse()?, + const_token: input.parse()?, + ident: input.parse()?, + colon_token: input.parse()?, + ty: input.parse()?, + semi_token: input.parse()?, + }) + } +} + pub(crate) fn make_const( field_ident: &Ident, ty: &Type, diff --git a/module-system/sov-modules-macros/tests/constants/create_constant.rs b/module-system/sov-modules-macros/tests/constants/create_constant.rs index f46240b1d..aecb5a227 100644 --- a/module-system/sov-modules-macros/tests/constants/create_constant.rs +++ b/module-system/sov-modules-macros/tests/constants/create_constant.rs @@ -1,18 +1,20 @@ use sov_modules_api::macros::config_constant; #[config_constant] -pub const TEST_U32: u32 = 0; +pub const TEST_U32: u32; #[config_constant] -pub const TEST_ARRAY_OF_U8: [u8; 32] = [0; 32]; +pub const TEST_ARRAY_OF_U8: [u8; 32]; #[config_constant] -pub const TEST_NESTED_ARRAY: [[u8; 3]; 2] = [[0; 3]; 2]; +/// This one has a doc attr +pub const TEST_NESTED_ARRAY: [[u8; 3]; 2]; #[config_constant] -pub const TEST_BOOL: bool = false; +pub const TEST_BOOL: bool; #[config_constant] -pub const TEST_STRING: &str = "Some String"; +/// This one is not visible +const TEST_STRING: &str; fn main() { assert_eq!(TEST_U32, 42);