From bcb355f4a0159d9f1eb9808b708a32b507af49e6 Mon Sep 17 00:00:00 2001 From: Jim Posen Date: Tue, 5 Nov 2019 23:54:55 +0100 Subject: [PATCH 1/4] Correctly serialize code in chain spec as hex. Due to a bug, the runtime code was previously serialized as a JSON array of numbers, pretty printed one byte per line. --- .../src/storage/genesis_config/genesis_config_def.rs | 11 +++++------ .../procedural/src/storage/genesis_config/mod.rs | 8 ++++---- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/srml/support/procedural/src/storage/genesis_config/genesis_config_def.rs b/srml/support/procedural/src/storage/genesis_config/genesis_config_def.rs index f9d2f8abe8055..259078164c98f 100644 --- a/srml/support/procedural/src/storage/genesis_config/genesis_config_def.rs +++ b/srml/support/procedural/src/storage/genesis_config/genesis_config_def.rs @@ -23,9 +23,9 @@ use quote::quote; use super::super::{DeclStorageDefExt, StorageLineTypeDef}; pub struct GenesisConfigFieldDef { - pub doc: Vec, pub name: syn::Ident, pub typ: syn::Type, + pub attrs: Vec, pub default: TokenStream, } @@ -114,17 +114,16 @@ impl GenesisConfigDef { .unwrap_or_else(|| quote!( Default::default() )); config_field_defs.push(GenesisConfigFieldDef { - doc: line.doc_attrs.clone(), name: config_field, typ, + attrs: line.doc_attrs.clone(), default, }); } for line in &def.extra_genesis_config_lines { - let doc = line.attrs.iter() - .filter_map(|a| a.parse_meta().ok()) - .filter(|m| m.name() == "doc") + let attrs = line.attrs.iter() + .map(|a| a.parse_meta().expect("attribute cannot be parsed")) .collect(); let default = line.default.as_ref().map(|e| quote!( #e )) @@ -132,9 +131,9 @@ impl GenesisConfigDef { config_field_defs.push(GenesisConfigFieldDef { - doc, name: line.name.clone(), typ: line.typ.clone(), + attrs, default, }); } diff --git a/srml/support/procedural/src/storage/genesis_config/mod.rs b/srml/support/procedural/src/storage/genesis_config/mod.rs index 2d4d4af3861ed..c17eb5dfe030b 100644 --- a/srml/support/procedural/src/storage/genesis_config/mod.rs +++ b/srml/support/procedural/src/storage/genesis_config/mod.rs @@ -33,13 +33,13 @@ fn decl_genesis_config_and_impl_default( genesis_config: &GenesisConfigDef, ) -> TokenStream { let config_fields = genesis_config.fields.iter().map(|field| { - let (name, typ, doc) = (&field.name, &field.typ, &field.doc); - quote!( #( #[ #doc] )* pub #name: #typ, ) + let (name, typ, attrs) = (&field.name, &field.typ, &field.attrs); + quote!( #( #[ #attrs] )* pub #name: #typ, ) }); let config_field_defaults = genesis_config.fields.iter().map(|field| { - let (name, default, doc) = (&field.name, &field.default, &field.doc); - quote!( #( #[ #doc] )* #name: #default, ) + let (name, default) = (&field.name, &field.default); + quote!( #name: #default, ) }); let serde_bug_bound = if !genesis_config.fields.is_empty() { From 61cd3f0f93fc32035db7ee1c9da0518132ecc91a Mon Sep 17 00:00:00 2001 From: Jim Posen Date: Wed, 6 Nov 2019 09:24:02 +0100 Subject: [PATCH 2/4] Remove panic in macro and whitelist attribute types for storage genesis config lines. --- .../src/storage/genesis_config/genesis_config_def.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/srml/support/procedural/src/storage/genesis_config/genesis_config_def.rs b/srml/support/procedural/src/storage/genesis_config/genesis_config_def.rs index 259078164c98f..839d7ca269239 100644 --- a/srml/support/procedural/src/storage/genesis_config/genesis_config_def.rs +++ b/srml/support/procedural/src/storage/genesis_config/genesis_config_def.rs @@ -123,7 +123,8 @@ impl GenesisConfigDef { for line in &def.extra_genesis_config_lines { let attrs = line.attrs.iter() - .map(|a| a.parse_meta().expect("attribute cannot be parsed")) + .filter_map(|a| a.parse_meta().ok()) + .filter(|m| m.name() == "doc" || m.name() == "serde") .collect(); let default = line.default.as_ref().map(|e| quote!( #e )) From 39199bb43bdebe1a9e21cf077a5cab42a623d319 Mon Sep 17 00:00:00 2001 From: Jim Posen Date: Thu, 7 Nov 2019 14:18:53 +0100 Subject: [PATCH 3/4] Use syn::Error to enforce whitelisted attributes on genesis config. --- .../genesis_config/genesis_config_def.rs | 31 +++++++++++++------ .../src/storage/genesis_config/mod.rs | 9 ++++-- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/srml/support/procedural/src/storage/genesis_config/genesis_config_def.rs b/srml/support/procedural/src/storage/genesis_config/genesis_config_def.rs index a3fe514bd638a..b86f6ca90c334 100644 --- a/srml/support/procedural/src/storage/genesis_config/genesis_config_def.rs +++ b/srml/support/procedural/src/storage/genesis_config/genesis_config_def.rs @@ -18,7 +18,7 @@ use srml_support_procedural_tools::syn_ext as ext; use proc_macro2::TokenStream; -use syn::parse_quote; +use syn::{spanned::Spanned, parse_quote}; use quote::quote; use super::super::{DeclStorageDefExt, StorageLineTypeDef}; @@ -43,8 +43,8 @@ pub struct GenesisConfigDef { } impl GenesisConfigDef { - pub fn from_def(def: &DeclStorageDefExt) -> Self { - let fields = Self::get_genesis_config_field_defs(def); + pub fn from_def(def: &DeclStorageDefExt) -> syn::Result { + let fields = Self::get_genesis_config_field_defs(def)?; let is_generic = fields.iter() .any(|field| ext::type_contains_ident(&field.typ, &def.module_runtime_generic)); @@ -71,17 +71,19 @@ impl GenesisConfigDef { (quote!(), quote!(), quote!(), None) }; - Self { + Ok(Self { is_generic, fields, genesis_struct_decl, genesis_struct, genesis_impl, genesis_where_clause, - } + }) } - fn get_genesis_config_field_defs(def: &DeclStorageDefExt) -> Vec { + fn get_genesis_config_field_defs(def: &DeclStorageDefExt) + -> syn::Result> + { let mut config_field_defs = Vec::new(); for (config_field, line) in def.storage_lines.iter() @@ -123,9 +125,18 @@ impl GenesisConfigDef { for line in &def.extra_genesis_config_lines { let attrs = line.attrs.iter() - .filter_map(|a| a.parse_meta().ok()) - .filter(|m| m.path().is_ident("doc") || m.path().is_ident("serde")) - .collect(); + .map(|attr| { + let meta = attr.parse_meta()?; + if meta.path().is_ident("doc") || meta.path().is_ident("serde") { + Ok(meta) + } else { + Err(syn::Error::new( + meta.span(), + "extra genesis config item only supports `doc` and `serde` attributes" + )) + } + }) + .collect::>()?; let default = line.default.as_ref().map(|e| quote!( #e )) .unwrap_or_else(|| quote!( Default::default() )); @@ -139,6 +150,6 @@ impl GenesisConfigDef { }); } - config_field_defs + Ok(config_field_defs) } } diff --git a/srml/support/procedural/src/storage/genesis_config/mod.rs b/srml/support/procedural/src/storage/genesis_config/mod.rs index c17eb5dfe030b..109957926a775 100644 --- a/srml/support/procedural/src/storage/genesis_config/mod.rs +++ b/srml/support/procedural/src/storage/genesis_config/mod.rs @@ -188,10 +188,13 @@ pub fn genesis_config_and_build_storage( ) -> TokenStream { let builders = BuilderDef::from_def(scrate, def); if !builders.blocks.is_empty() { - let genesis_config = &GenesisConfigDef::from_def(def); + let genesis_config = match GenesisConfigDef::from_def(def) { + Ok(genesis_config) => genesis_config, + Err(err) => return err.to_compile_error(), + }; let decl_genesis_config_and_impl_default = - decl_genesis_config_and_impl_default(scrate, genesis_config); - let impl_build_storage = impl_build_storage(scrate, def, genesis_config, &builders); + decl_genesis_config_and_impl_default(scrate, &genesis_config); + let impl_build_storage = impl_build_storage(scrate, def, &genesis_config, &builders); quote!{ #decl_genesis_config_and_impl_default From 3d01bc4acc96e336c6368c6571f1e1dcd81df380 Mon Sep 17 00:00:00 2001 From: Jim Posen Date: Thu, 7 Nov 2019 15:12:40 +0100 Subject: [PATCH 4/4] Blacklist genesis extra config line attributes instead of whitelist. --- .../src/storage/genesis_config/genesis_config_def.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/srml/support/procedural/src/storage/genesis_config/genesis_config_def.rs b/srml/support/procedural/src/storage/genesis_config/genesis_config_def.rs index b86f6ca90c334..4bf665de71fa6 100644 --- a/srml/support/procedural/src/storage/genesis_config/genesis_config_def.rs +++ b/srml/support/procedural/src/storage/genesis_config/genesis_config_def.rs @@ -127,14 +127,13 @@ impl GenesisConfigDef { let attrs = line.attrs.iter() .map(|attr| { let meta = attr.parse_meta()?; - if meta.path().is_ident("doc") || meta.path().is_ident("serde") { - Ok(meta) - } else { - Err(syn::Error::new( + if meta.path().is_ident("cfg") { + return Err(syn::Error::new( meta.span(), - "extra genesis config item only supports `doc` and `serde` attributes" - )) + "extra genesis config items do not support `cfg` attribute" + )); } + Ok(meta) }) .collect::>()?;