From 83ca67d1e6b14d068603f0914b4b79ff5c4b38c0 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Wed, 10 May 2023 20:48:56 +0530 Subject: [PATCH 1/9] Initial setup --- .../procedural/src/pallet/parse/storage.rs | 53 +++++++++++++++---- ...explicit_key_without_arg_default_hasher.rs | 33 ++++++++++++ ...icit_key_without_arg_default_hasher.stderr | 11 ++++ .../tests/pallet_ui/pass/dev_mode_valid.rs | 9 ++++ 4 files changed, 95 insertions(+), 11 deletions(-) create mode 100644 frame/support/test/tests/pallet_ui/dev_mode_explicit_key_without_arg_default_hasher.rs create mode 100644 frame/support/test/tests/pallet_ui/dev_mode_explicit_key_without_arg_default_hasher.stderr diff --git a/frame/support/procedural/src/pallet/parse/storage.rs b/frame/support/procedural/src/pallet/parse/storage.rs index 16c0851ce7a04..a6bac4e505e8c 100644 --- a/frame/support/procedural/src/pallet/parse/storage.rs +++ b/frame/support/procedural/src/pallet/parse/storage.rs @@ -332,6 +332,7 @@ fn process_named_generics( storage: &StorageKind, args_span: proc_macro2::Span, args: &[syn::AssocType], + dev_mode: bool, ) -> syn::Result<(Option, Metadata, Option, bool)> { let mut parsed = HashMap::::new(); @@ -366,10 +367,20 @@ fn process_named_generics( } }, StorageKind::Map => { + let mandatory_generics: &[&str]; + let optional_generics: &[&str]; + if dev_mode { + mandatory_generics = &["Key", "Value"]; + optional_generics = &["Hasher", "QueryKind", "OnEmpty", "MaxValues"]; + } else { + mandatory_generics = &["Hasher", "Key", "Value"]; + optional_generics = &["QueryKind", "OnEmpty", "MaxValues"]; + } + check_generics( &parsed, - &["Hasher", "Key", "Value"], - &["QueryKind", "OnEmpty", "MaxValues"], + mandatory_generics, + optional_generics, "StorageMap", args_span, )?; @@ -378,7 +389,7 @@ fn process_named_generics( hasher: parsed .remove("Hasher") .map(|binding| binding.ty) - .expect("checked above as mandatory generic"), + .unwrap_or(syn::Type::Verbatim(quote::quote! { Blake2_128Concat })), key: parsed .remove("Key") .map(|binding| binding.ty) @@ -393,10 +404,20 @@ fn process_named_generics( } }, StorageKind::CountedMap => { + let mandatory_generics: &[&str]; + let optional_generics: &[&str]; + if dev_mode { + mandatory_generics = &["Key", "Value"]; + optional_generics = &["Hasher", "QueryKind", "OnEmpty", "MaxValues"]; + } else { + mandatory_generics = &["Hasher", "Key", "Value"]; + optional_generics = &["QueryKind", "OnEmpty", "MaxValues"]; + } + check_generics( &parsed, - &["Hasher", "Key", "Value"], - &["QueryKind", "OnEmpty", "MaxValues"], + mandatory_generics, + optional_generics, "CountedStorageMap", args_span, )?; @@ -405,7 +426,7 @@ fn process_named_generics( hasher: parsed .remove("Hasher") .map(|binding| binding.ty) - .expect("checked above as mandatory generic"), + .unwrap_or(syn::Type::Verbatim(quote::quote! { Blake2_128Concat })), key: parsed .remove("Key") .map(|binding| binding.ty) @@ -420,10 +441,20 @@ fn process_named_generics( } }, StorageKind::DoubleMap => { + let mandatory_generics: &[&str]; + let optional_generics: &[&str]; + if dev_mode { + mandatory_generics = &["Key1", "Key2", "Value"]; + optional_generics = &["Hasher1", "Hasher2", "QueryKind", "OnEmpty", "MaxValues"]; + } else { + mandatory_generics = &["Hasher1", "Key1", "Hasher2", "Key2", "Value"]; + optional_generics = &["QueryKind", "OnEmpty", "MaxValues"]; + } + check_generics( &parsed, - &["Hasher1", "Key1", "Hasher2", "Key2", "Value"], - &["QueryKind", "OnEmpty", "MaxValues"], + mandatory_generics, + optional_generics, "StorageDoubleMap", args_span, )?; @@ -432,7 +463,7 @@ fn process_named_generics( hasher1: parsed .remove("Hasher1") .map(|binding| binding.ty) - .expect("checked above as mandatory generic"), + .unwrap_or(syn::Type::Verbatim(quote::quote! { Blake2_128Concat })), key1: parsed .remove("Key1") .map(|binding| binding.ty) @@ -440,7 +471,7 @@ fn process_named_generics( hasher2: parsed .remove("Hasher2") .map(|binding| binding.ty) - .expect("checked above as mandatory generic"), + .unwrap_or(syn::Type::Verbatim(quote::quote! { Blake2_128Concat })), key2: parsed .remove("Key2") .map(|binding| binding.ty) @@ -619,7 +650,7 @@ fn process_generics( _ => unreachable!("It is asserted above that all generics are bindings"), }) .collect::>(); - process_named_generics(&storage_kind, args_span, &args) + process_named_generics(&storage_kind, args_span, &args, dev_mode) } else { let msg = "Invalid pallet::storage, invalid generic declaration for storage. Expect only \ type generics or binding generics, e.g. `` or \ diff --git a/frame/support/test/tests/pallet_ui/dev_mode_explicit_key_without_arg_default_hasher.rs b/frame/support/test/tests/pallet_ui/dev_mode_explicit_key_without_arg_default_hasher.rs new file mode 100644 index 0000000000000..7d8be8ec00174 --- /dev/null +++ b/frame/support/test/tests/pallet_ui/dev_mode_explicit_key_without_arg_default_hasher.rs @@ -0,0 +1,33 @@ +#![cfg_attr(not(feature = "std"), no_std)] + +pub use pallet::*; + +#[frame_support::pallet] +pub mod pallet { + use frame_support::pallet_prelude::*; + + // The struct on which we build all of our Pallet logic. + #[pallet::pallet] + pub struct Pallet(_); + + // Your Pallet's configuration trait, representing custom external types and interfaces. + #[pallet::config] + pub trait Config: frame_system::Config {} + + #[pallet::storage] + type MyStorage = StorageValue<_, Vec>; + + #[pallet::storage] + type MyStorageMap = StorageMap; + + #[pallet::storage] + type MyStorageDoubleMap = StorageDoubleMap; + + #[pallet::storage] + type MyCountedStorageMap = CountedStorageMap; + + // Your Pallet's internal functions. + impl Pallet {} +} + +fn main() {} diff --git a/frame/support/test/tests/pallet_ui/dev_mode_explicit_key_without_arg_default_hasher.stderr b/frame/support/test/tests/pallet_ui/dev_mode_explicit_key_without_arg_default_hasher.stderr new file mode 100644 index 0000000000000..171017396a2cd --- /dev/null +++ b/frame/support/test/tests/pallet_ui/dev_mode_explicit_key_without_arg_default_hasher.stderr @@ -0,0 +1,11 @@ +error: Invalid pallet::storage, cannot find `Hasher` generic, required for `StorageMap`. + --> tests/pallet_ui/dev_mode_explicit_key_without_arg_default_hasher.rs:21:43 + | +21 | type MyStorageMap = StorageMap; + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0432]: unresolved import `pallet` + --> tests/pallet_ui/dev_mode_explicit_key_without_arg_default_hasher.rs:3:9 + | +3 | pub use pallet::*; + | ^^^^^^ help: a similar path exists: `test_pallet::pallet` diff --git a/frame/support/test/tests/pallet_ui/pass/dev_mode_valid.rs b/frame/support/test/tests/pallet_ui/pass/dev_mode_valid.rs index 483ed9579bd0a..cd2d9ed6cc399 100644 --- a/frame/support/test/tests/pallet_ui/pass/dev_mode_valid.rs +++ b/frame/support/test/tests/pallet_ui/pass/dev_mode_valid.rs @@ -35,6 +35,15 @@ pub mod pallet { #[pallet::storage] type MyCountedStorageMap = CountedStorageMap<_, _, u32, u64>; + #[pallet::storage] + pub type MyStorageMap2 = StorageMap; + + #[pallet::storage] + type MyStorageDoubleMap = StorageDoubleMap; + + #[pallet::storage] + type MyCountedStorageMap = CountedStorageMap; + // Your Pallet's callable functions. #[pallet::call] impl Pallet { From 9894ee6217b0dc852e022ad6c180969caa24f5ad Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Wed, 17 May 2023 11:35:11 +0530 Subject: [PATCH 2/9] Minor update --- frame/support/test/tests/pallet_ui/pass/dev_mode_valid.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/support/test/tests/pallet_ui/pass/dev_mode_valid.rs b/frame/support/test/tests/pallet_ui/pass/dev_mode_valid.rs index cd2d9ed6cc399..28b901213943c 100644 --- a/frame/support/test/tests/pallet_ui/pass/dev_mode_valid.rs +++ b/frame/support/test/tests/pallet_ui/pass/dev_mode_valid.rs @@ -39,10 +39,10 @@ pub mod pallet { pub type MyStorageMap2 = StorageMap; #[pallet::storage] - type MyStorageDoubleMap = StorageDoubleMap; + type MyStorageDoubleMap2 = StorageDoubleMap; #[pallet::storage] - type MyCountedStorageMap = CountedStorageMap; + type MyCountedStorageMap2 = CountedStorageMap; // Your Pallet's callable functions. #[pallet::call] From 6c2ed0b60a69ca37ef670bda4d53e2665799d0b9 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Wed, 17 May 2023 12:00:11 +0530 Subject: [PATCH 3/9] Minor update --- .../dev_mode_explicit_key_without_arg_default_hasher.stderr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/test/tests/pallet_ui/dev_mode_explicit_key_without_arg_default_hasher.stderr b/frame/support/test/tests/pallet_ui/dev_mode_explicit_key_without_arg_default_hasher.stderr index 171017396a2cd..b93d5c878e919 100644 --- a/frame/support/test/tests/pallet_ui/dev_mode_explicit_key_without_arg_default_hasher.stderr +++ b/frame/support/test/tests/pallet_ui/dev_mode_explicit_key_without_arg_default_hasher.stderr @@ -2,7 +2,7 @@ error: Invalid pallet::storage, cannot find `Hasher` generic, required for `Stor --> tests/pallet_ui/dev_mode_explicit_key_without_arg_default_hasher.rs:21:43 | 21 | type MyStorageMap = StorageMap; - | ^^^^^^^^^^^^^^^^^^^^^^^^ + | ^ error[E0432]: unresolved import `pallet` --> tests/pallet_ui/dev_mode_explicit_key_without_arg_default_hasher.rs:3:9 From 3ea501ebff98e3f0339196d5b5867a70bb3c2682 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Wed, 17 May 2023 19:57:58 +0530 Subject: [PATCH 4/9] Addresses review comments --- frame/support/procedural/src/pallet/parse/storage.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/support/procedural/src/pallet/parse/storage.rs b/frame/support/procedural/src/pallet/parse/storage.rs index a6bac4e505e8c..4ef3dd1b523b4 100644 --- a/frame/support/procedural/src/pallet/parse/storage.rs +++ b/frame/support/procedural/src/pallet/parse/storage.rs @@ -389,7 +389,7 @@ fn process_named_generics( hasher: parsed .remove("Hasher") .map(|binding| binding.ty) - .unwrap_or(syn::Type::Verbatim(quote::quote! { Blake2_128Concat })), + .unwrap_or(syn::parse_quote!(Blake2_128Concat)), key: parsed .remove("Key") .map(|binding| binding.ty) @@ -463,7 +463,7 @@ fn process_named_generics( hasher1: parsed .remove("Hasher1") .map(|binding| binding.ty) - .unwrap_or(syn::Type::Verbatim(quote::quote! { Blake2_128Concat })), + .unwrap_or(syn::parse_quote!(Blake2_128Concat)), key1: parsed .remove("Key1") .map(|binding| binding.ty) @@ -471,7 +471,7 @@ fn process_named_generics( hasher2: parsed .remove("Hasher2") .map(|binding| binding.ty) - .unwrap_or(syn::Type::Verbatim(quote::quote! { Blake2_128Concat })), + .unwrap_or(syn::parse_quote!(Blake2_128Concat)), key2: parsed .remove("Key2") .map(|binding| binding.ty) From 54a95c676d3d887e797ea51f81b5bf17c5f66540 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Thu, 18 May 2023 14:42:00 +0530 Subject: [PATCH 5/9] Addresses review comments --- .../procedural/src/pallet/parse/storage.rs | 49 +++++++------------ 1 file changed, 17 insertions(+), 32 deletions(-) diff --git a/frame/support/procedural/src/pallet/parse/storage.rs b/frame/support/procedural/src/pallet/parse/storage.rs index 4ef3dd1b523b4..12e06b214b6b6 100644 --- a/frame/support/procedural/src/pallet/parse/storage.rs +++ b/frame/support/procedural/src/pallet/parse/storage.rs @@ -347,6 +347,14 @@ fn process_named_generics( parsed.insert(arg.ident.to_string(), arg.clone()); } + let mut map_mandatory_generics = vec!["Key", "Value"]; + let mut map_optional_generics = vec!["QueryKind", "OnEmpty", "MaxValues"]; + if dev_mode { + map_optional_generics.push("Hasher"); + } else { + map_mandatory_generics.push("Hasher"); + } + let generics = match storage { StorageKind::Value => { check_generics( @@ -367,20 +375,10 @@ fn process_named_generics( } }, StorageKind::Map => { - let mandatory_generics: &[&str]; - let optional_generics: &[&str]; - if dev_mode { - mandatory_generics = &["Key", "Value"]; - optional_generics = &["Hasher", "QueryKind", "OnEmpty", "MaxValues"]; - } else { - mandatory_generics = &["Hasher", "Key", "Value"]; - optional_generics = &["QueryKind", "OnEmpty", "MaxValues"]; - } - check_generics( &parsed, - mandatory_generics, - optional_generics, + &map_mandatory_generics, + &map_optional_generics, "StorageMap", args_span, )?; @@ -404,20 +402,10 @@ fn process_named_generics( } }, StorageKind::CountedMap => { - let mandatory_generics: &[&str]; - let optional_generics: &[&str]; - if dev_mode { - mandatory_generics = &["Key", "Value"]; - optional_generics = &["Hasher", "QueryKind", "OnEmpty", "MaxValues"]; - } else { - mandatory_generics = &["Hasher", "Key", "Value"]; - optional_generics = &["QueryKind", "OnEmpty", "MaxValues"]; - } - check_generics( &parsed, - mandatory_generics, - optional_generics, + &map_mandatory_generics, + &map_optional_generics, "CountedStorageMap", args_span, )?; @@ -441,20 +429,17 @@ fn process_named_generics( } }, StorageKind::DoubleMap => { - let mandatory_generics: &[&str]; - let optional_generics: &[&str]; + let mut double_map_mandatory_generics = vec!["Key1", "Key2", "Value"]; if dev_mode { - mandatory_generics = &["Key1", "Key2", "Value"]; - optional_generics = &["Hasher1", "Hasher2", "QueryKind", "OnEmpty", "MaxValues"]; + map_optional_generics.extend(["Hasher1", "Hasher2"]); } else { - mandatory_generics = &["Hasher1", "Key1", "Hasher2", "Key2", "Value"]; - optional_generics = &["QueryKind", "OnEmpty", "MaxValues"]; + double_map_mandatory_generics.extend(["Hasher1", "Hasher2"]); } check_generics( &parsed, - mandatory_generics, - optional_generics, + &double_map_mandatory_generics, + &map_optional_generics, "StorageDoubleMap", args_span, )?; From 6dd6fc8d15d922f379325f4118c6b38d7042685d Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Fri, 19 May 2023 09:34:43 +0530 Subject: [PATCH 6/9] Updates doc --- frame/support/procedural/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index b8aa5674ddde5..740eecb666f88 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -468,7 +468,8 @@ pub fn construct_runtime(input: TokenStream) -> TokenStream { /// storage types. This is equivalent to specifying `#[pallet::unbounded]` on all storage type /// definitions. /// * Storage hashers no longer need to be specified and can be replaced by `_`. In dev mode, these -/// will be replaced by `Blake2_128Concat`. +/// will be replaced by `Blake2_128Concat`. In case of explicit key-binding, `Hasher` can simply be +/// ignored when in `dev_mode`. /// /// Note that the `dev_mode` argument can only be supplied to the `#[pallet]` or /// `#[frame_support::pallet]` attribute macro that encloses your pallet module. This argument From 85eeddaba83cd3d2a6e990162fdc944908ef6cf8 Mon Sep 17 00:00:00 2001 From: command-bot <> Date: Fri, 19 May 2023 04:15:53 +0000 Subject: [PATCH 7/9] ".git/.scripts/commands/fmt/fmt.sh" --- frame/support/procedural/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index 740eecb666f88..25df8410b02d1 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -468,8 +468,8 @@ pub fn construct_runtime(input: TokenStream) -> TokenStream { /// storage types. This is equivalent to specifying `#[pallet::unbounded]` on all storage type /// definitions. /// * Storage hashers no longer need to be specified and can be replaced by `_`. In dev mode, these -/// will be replaced by `Blake2_128Concat`. In case of explicit key-binding, `Hasher` can simply be -/// ignored when in `dev_mode`. +/// will be replaced by `Blake2_128Concat`. In case of explicit key-binding, `Hasher` can simply +/// be ignored when in `dev_mode`. /// /// Note that the `dev_mode` argument can only be supplied to the `#[pallet]` or /// `#[frame_support::pallet]` attribute macro that encloses your pallet module. This argument From 4ce0e38ae2e7a44e205a8e7da285f758445471ac Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Fri, 19 May 2023 17:57:36 +0530 Subject: [PATCH 8/9] Renames file --- ...rs => non_dev_mode_storage_map_explicit_key_default_hasher.rs} | 0 ...> non_dev_mode_storage_map_explicit_key_default_hasher.stderr} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename frame/support/test/tests/pallet_ui/{dev_mode_explicit_key_without_arg_default_hasher.rs => non_dev_mode_storage_map_explicit_key_default_hasher.rs} (100%) rename frame/support/test/tests/pallet_ui/{dev_mode_explicit_key_without_arg_default_hasher.stderr => non_dev_mode_storage_map_explicit_key_default_hasher.stderr} (100%) diff --git a/frame/support/test/tests/pallet_ui/dev_mode_explicit_key_without_arg_default_hasher.rs b/frame/support/test/tests/pallet_ui/non_dev_mode_storage_map_explicit_key_default_hasher.rs similarity index 100% rename from frame/support/test/tests/pallet_ui/dev_mode_explicit_key_without_arg_default_hasher.rs rename to frame/support/test/tests/pallet_ui/non_dev_mode_storage_map_explicit_key_default_hasher.rs diff --git a/frame/support/test/tests/pallet_ui/dev_mode_explicit_key_without_arg_default_hasher.stderr b/frame/support/test/tests/pallet_ui/non_dev_mode_storage_map_explicit_key_default_hasher.stderr similarity index 100% rename from frame/support/test/tests/pallet_ui/dev_mode_explicit_key_without_arg_default_hasher.stderr rename to frame/support/test/tests/pallet_ui/non_dev_mode_storage_map_explicit_key_default_hasher.stderr From 2e60975e4f35591382a9129f3dd9f8c62b6c0c77 Mon Sep 17 00:00:00 2001 From: Nikhil Gupta <17176722+gupnik@users.noreply.github.com> Date: Fri, 19 May 2023 18:08:34 +0530 Subject: [PATCH 9/9] Updates path in test --- ...on_dev_mode_storage_map_explicit_key_default_hasher.stderr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/support/test/tests/pallet_ui/non_dev_mode_storage_map_explicit_key_default_hasher.stderr b/frame/support/test/tests/pallet_ui/non_dev_mode_storage_map_explicit_key_default_hasher.stderr index b93d5c878e919..68751470a3e2f 100644 --- a/frame/support/test/tests/pallet_ui/non_dev_mode_storage_map_explicit_key_default_hasher.stderr +++ b/frame/support/test/tests/pallet_ui/non_dev_mode_storage_map_explicit_key_default_hasher.stderr @@ -1,11 +1,11 @@ error: Invalid pallet::storage, cannot find `Hasher` generic, required for `StorageMap`. - --> tests/pallet_ui/dev_mode_explicit_key_without_arg_default_hasher.rs:21:43 + --> tests/pallet_ui/non_dev_mode_storage_map_explicit_key_default_hasher.rs:21:43 | 21 | type MyStorageMap = StorageMap; | ^ error[E0432]: unresolved import `pallet` - --> tests/pallet_ui/dev_mode_explicit_key_without_arg_default_hasher.rs:3:9 + --> tests/pallet_ui/non_dev_mode_storage_map_explicit_key_default_hasher.rs:3:9 | 3 | pub use pallet::*; | ^^^^^^ help: a similar path exists: `test_pallet::pallet`