Makes storage hashers optional in dev mode (#13815)

* Initial changes

* Adds UI test for error when _ is used without dev_mode

* Minor

* ".git/.scripts/commands/fmt/fmt.sh"

* Adds test to verify hasher

---------

Co-authored-by: command-bot <>

frame/support/procedural/src/pallet/expand/storage.rs

@@ -269,6 +269,19 @@ pub fn process_generics(def: &mut Def) -> syn::Result<Vec<ResultOnEmptyStructMet
 				Metadata::DoubleMap { .. } => (5, 6, 7),
 			};
 
+			if storage_def.use_default_hasher {
+				let hasher_indices: Vec<usize> = match storage_def.metadata {
+					Metadata::Map { .. } | Metadata::CountedMap { .. } => vec![1],
+					Metadata::DoubleMap { .. } => vec![1, 3],
+					_ => vec![],
+				};
+				for hasher_idx in hasher_indices {
+					args.args[hasher_idx] = syn::GenericArgument::Type(
+						syn::parse_quote!(#frame_support::Blake2_128Concat),
+					);
+				}
+			}
+
 			if query_idx < args.args.len() {
 				if let syn::GenericArgument::Type(query_kind) = args.args.index_mut(query_idx) {
 					set_result_query_type_parameter(query_kind)?;

frame/support/procedural/src/pallet/parse/storage.rs

@@ -185,6 +185,8 @@ pub struct StorageDef {
 	pub unbounded: bool,
 	/// Whether or not reads to this storage key will be ignored by benchmarking
 	pub whitelisted: bool,
+	/// Whether or not a default hasher is allowed to replace `_`
+	pub use_default_hasher: bool,
 }
 
 /// The parsed generic from the
@@ -325,12 +327,12 @@ fn check_generics(
 	}
 }
 
-/// Returns `(named generics, metadata, query kind)`
+/// Returns `(named generics, metadata, query kind, use_default_hasher)`
 fn process_named_generics(
 	storage: &StorageKind,
 	args_span: proc_macro2::Span,
 	args: &[syn::Binding],
-) -> syn::Result<(Option<StorageGenerics>, Metadata, Option<syn::Type>)> {
+) -> syn::Result<(Option<StorageGenerics>, Metadata, Option<syn::Type>, bool)> {
 	let mut parsed = HashMap::<String, syn::Binding>::new();
 
 	// Ensure no duplicate.
@@ -480,15 +482,16 @@ fn process_named_generics(
 	let metadata = generics.metadata()?;
 	let query_kind = generics.query_kind();
 
-	Ok((Some(generics), metadata, query_kind))
+	Ok((Some(generics), metadata, query_kind, false))
 }
 
-/// Returns `(named generics, metadata, query kind)`
+/// Returns `(named generics, metadata, query kind, use_default_hasher)`
 fn process_unnamed_generics(
 	storage: &StorageKind,
 	args_span: proc_macro2::Span,
 	args: &[syn::Type],
-) -> syn::Result<(Option<StorageGenerics>, Metadata, Option<syn::Type>)> {
+	dev_mode: bool,
+) -> syn::Result<(Option<StorageGenerics>, Metadata, Option<syn::Type>, bool)> {
 	let retrieve_arg = |arg_pos| {
 		args.get(arg_pos).cloned().ok_or_else(|| {
 			let msg = format!(
@@ -510,18 +513,28 @@ fn process_unnamed_generics(
 		err
 	})?;
 
+	let use_default_hasher = |arg_pos| {
+		if let Some(arg) = retrieve_arg(arg_pos).ok() {
+			dev_mode && syn::parse2::<syn::Token![_]>(arg.to_token_stream()).is_ok()
+		} else {
+			false
+		}
+	};
+
 	let res = match storage {
 		StorageKind::Value =>
-			(None, Metadata::Value { value: retrieve_arg(1)? }, retrieve_arg(2).ok()),
+			(None, Metadata::Value { value: retrieve_arg(1)? }, retrieve_arg(2).ok(), false),
 		StorageKind::Map => (
 			None,
 			Metadata::Map { key: retrieve_arg(2)?, value: retrieve_arg(3)? },
 			retrieve_arg(4).ok(),
+			use_default_hasher(1),
 		),
 		StorageKind::CountedMap => (
 			None,
 			Metadata::CountedMap { key: retrieve_arg(2)?, value: retrieve_arg(3)? },
 			retrieve_arg(4).ok(),
+			use_default_hasher(1),
 		),
 		StorageKind::DoubleMap => (
 			None,
@@ -531,21 +544,28 @@ fn process_unnamed_generics(
 				value: retrieve_arg(5)?,
 			},
 			retrieve_arg(6).ok(),
+			use_default_hasher(1) && use_default_hasher(3),
 		),
 		StorageKind::NMap => {
 			let keygen = retrieve_arg(1)?;
 			let keys = collect_keys(&keygen)?;
-			(None, Metadata::NMap { keys, keygen, value: retrieve_arg(2)? }, retrieve_arg(3).ok())
+			(
+				None,
+				Metadata::NMap { keys, keygen, value: retrieve_arg(2)? },
+				retrieve_arg(3).ok(),
+				false,
+			)
 		},
 	};
 
 	Ok(res)
 }
 
-/// Returns `(named generics, metadata, query kind)`
+/// Returns `(named generics, metadata, query kind, use_default_hasher)`
 fn process_generics(
 	segment: &syn::PathSegment,
-) -> syn::Result<(Option<StorageGenerics>, Metadata, Option<syn::Type>)> {
+	dev_mode: bool,
+) -> syn::Result<(Option<StorageGenerics>, Metadata, Option<syn::Type>, bool)> {
 	let storage_kind = match &*segment.ident.to_string() {
 		"StorageValue" => StorageKind::Value,
 		"StorageMap" => StorageKind::Map,
@@ -583,7 +603,7 @@ fn process_generics(
 				_ => unreachable!("It is asserted above that all generics are types"),
 			})
 			.collect::<Vec<_>>();
-		process_unnamed_generics(&storage_kind, args_span, &args)
+		process_unnamed_generics(&storage_kind, args_span, &args, dev_mode)
 	} else if args.args.iter().all(|gen| matches!(gen, syn::GenericArgument::Binding(_))) {
 		let args = args
 			.args
@@ -711,7 +731,8 @@ impl StorageDef {
 			return Err(syn::Error::new(item.ty.span(), msg))
 		}
 
-		let (named_generics, metadata, query_kind) = process_generics(&typ.path.segments[0])?;
+		let (named_generics, metadata, query_kind, use_default_hasher) =
+			process_generics(&typ.path.segments[0], dev_mode)?;
 
 		let query_kind = query_kind
 			.map(|query_kind| {
@@ -832,6 +853,7 @@ impl StorageDef {
 			named_generics,
 			unbounded,
 			whitelisted,
+			use_default_hasher,
 		})
 	}
 }

frame/support/test/tests/pallet_ui/dev_mode_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<T>(_);
+
+	// Your Pallet's configuration trait, representing custom external types and interfaces.
+	#[pallet::config]
+	pub trait Config: frame_system::Config {}
+
+	#[pallet::storage]
+	type MyStorage<T: Config> = StorageValue<_, Vec<u8>>;
+
+	#[pallet::storage]
+	type MyStorageMap<T: Config> = StorageMap<_, _, u32, u64>;
+
+	#[pallet::storage]
+	type MyStorageDoubleMap<T: Config> = StorageDoubleMap<_, _, u32, _, u64, u64>;
+
+	#[pallet::storage]
+	type MyCountedStorageMap<T: Config> = CountedStorageMap<_, _, u32, u64>;
+
+	// Your Pallet's internal functions.
+	impl<T: Config> Pallet<T> {}
+}
+
+fn main() {}

frame/support/test/tests/pallet_ui/dev_mode_without_arg_default_hasher.stderr

@@ -0,0 +1,19 @@
+error[E0121]: the placeholder `_` is not allowed within types on item signatures for type aliases
+  --> tests/pallet_ui/dev_mode_without_arg_default_hasher.rs:21:47
+   |
+21 |     type MyStorageMap<T: Config> = StorageMap<_, _, u32, u64>;
+   |                                                  ^ not allowed in type signatures
+
+error[E0121]: the placeholder `_` is not allowed within types on item signatures for type aliases
+  --> tests/pallet_ui/dev_mode_without_arg_default_hasher.rs:24:59
+   |
+24 |     type MyStorageDoubleMap<T: Config> = StorageDoubleMap<_, _, u32, _, u64, u64>;
+   |                                                              ^       ^ not allowed in type signatures
+   |                                                              |
+   |                                                              not allowed in type signatures
+
+error[E0121]: the placeholder `_` is not allowed within types on item signatures for type aliases
+  --> tests/pallet_ui/dev_mode_without_arg_default_hasher.rs:27:61
+   |
+27 |     type MyCountedStorageMap<T: Config> = CountedStorageMap<_, _, u32, u64>;
+   |                                                                ^ not allowed in type signatures

frame/support/test/tests/pallet_ui/pass/dev_mode_valid.rs

@@ -1,5 +1,11 @@
 #![cfg_attr(not(feature = "std"), no_std)]
 
+use frame_support::{
+	traits::{
+		ConstU32,
+	},
+};
+
 pub use pallet::*;
 
 #[frame_support::pallet(dev_mode)]
@@ -19,6 +25,16 @@ pub mod pallet {
 	#[pallet::storage]
 	type MyStorage<T: Config> = StorageValue<_, Vec<u8>>;
 
+	// The Hasher requirement skipped by `dev_mode`.
+	#[pallet::storage]
+	pub type MyStorageMap<T: Config> = StorageMap<_, _, u32, u64>;
+
+	#[pallet::storage]
+	type MyStorageDoubleMap<T: Config> = StorageDoubleMap<_, _, u32, _, u64, u64>;
+
+	#[pallet::storage]
+	type MyCountedStorageMap<T: Config> = CountedStorageMap<_, _, u32, u64>;
+
 	// Your Pallet's callable functions.
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {
@@ -32,4 +48,71 @@ pub mod pallet {
 	impl<T: Config> Pallet<T> {}
 }
 
-fn main() {}
+impl frame_system::Config for Runtime {
+	type BaseCallFilter = frame_support::traits::Everything;
+	type RuntimeOrigin = RuntimeOrigin;
+	type Index = u64;
+	type BlockNumber = u32;
+	type RuntimeCall = RuntimeCall;
+	type Hash = sp_runtime::testing::H256;
+	type Hashing = sp_runtime::traits::BlakeTwo256;
+	type AccountId = u64;
+	type Lookup = sp_runtime::traits::IdentityLookup<Self::AccountId>;
+	type Header = Header;
+	type RuntimeEvent = RuntimeEvent;
+	type BlockHashCount = ConstU32<250>;
+	type BlockWeights = ();
+	type BlockLength = ();
+	type DbWeight = ();
+	type Version = ();
+	type PalletInfo = PalletInfo;
+	type AccountData = ();
+	type OnNewAccount = ();
+	type OnKilledAccount = ();
+	type SystemWeightInfo = ();
+	type SS58Prefix = ();
+	type OnSetCode = ();
+	type MaxConsumers = ConstU32<16>;
+}
+
+pub type Header = sp_runtime::generic::Header<u32, sp_runtime::traits::BlakeTwo256>;
+pub type Block = sp_runtime::generic::Block<Header, UncheckedExtrinsic>;
+pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic<u32, RuntimeCall, (), ()>;
+
+frame_support::construct_runtime!(
+	pub struct Runtime where
+		Block = Block,
+		NodeBlock = Block,
+		UncheckedExtrinsic = UncheckedExtrinsic
+	{
+		// Exclude part `Storage` in order not to check its metadata in tests.
+		System: frame_system exclude_parts { Pallet, Storage },
+		Example: pallet,
+	}
+);
+
+impl pallet::Config for Runtime {
+
+}
+
+fn main() {
+	use frame_support::{pallet_prelude::*};
+	use storage::unhashed;
+	use sp_io::{
+		hashing::{blake2_128, twox_128},
+		TestExternalities,
+	};
+
+	fn blake2_128_concat(d: &[u8]) -> Vec<u8> {
+		let mut v = blake2_128(d).to_vec();
+		v.extend_from_slice(d);
+		v
+	}
+
+	TestExternalities::default().execute_with(|| {
+		pallet::MyStorageMap::<Runtime>::insert(1, 2);
+		let mut k = [twox_128(b"Example"), twox_128(b"MyStorageMap")].concat();
+		k.extend(1u32.using_encoded(blake2_128_concat));
+		assert_eq!(unhashed::get::<u64>(&k), Some(2u64));
+	});
+}