Add log and warning about inserting empty values in trie.

frame/support/procedural/src/lib.rs

@@ -50,6 +50,9 @@ use proc_macro::TokenStream;
 /// prefix. Instance prefix is "" for default instance and "Instance$n" for instance number $n.
 /// Thus, instance 3 of module Example has a module prefix of `Instance3Example`
 ///
+/// **Warning**: storage doesn't support inserting empty values, so any value inserted must encode
+/// to a non-empty slice.
+///
 /// Basic storage consists of a name and a type; supported types are:
 ///
 /// * Value: `Foo: type`: Implements the
@@ -227,11 +230,15 @@ use proc_macro::TokenStream;
 /// add_extra_genesis {
 /// 	config(phantom): std::marker::PhantomData<I>,
 /// }
-/// ...
+/// ```
 ///
 /// This adds a field to your `GenesisConfig` with the name `phantom` that you can initialize with
 /// `Default::default()`.
 ///
+/// # Warning
+///
+/// storage doesn't support inserting empty values, so any value inserted must encode to a
+/// non-empty slice.
 #[proc_macro]
 pub fn decl_storage(input: TokenStream) -> TokenStream {
 	storage::decl_storage_impl(input)

frame/support/procedural/src/storage/empty_value_test.rs

@@ -0,0 +1,71 @@
+// This file is part of Substrate.
+
+// Copyright (C) 2020 Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//! Implementation of some test. Test assert default value (for storage with default value) doesn't
+//! encode to an empty slice.
+
+use proc_macro2::TokenStream;
+use quote::quote;
+use syn::spanned::Spanned;
+use super::DeclStorageDefExt;
+
+pub fn impl_empty_value_test(scrate: &TokenStream, def: &DeclStorageDefExt) -> TokenStream {
+	// If there is `Option<()>` or `()` return compile error
+	let unit_type = syn::Type::Tuple(syn::TypeTuple {
+		paren_token: Default::default(),
+		elems: syn::punctuated::Punctuated::new(),
+	});
+	for line in &def.storage_lines {
+		if line.value_type == unit_type {
+			return syn::Error::new(
+				line.value_type.span(),
+				"`()` is not supported, because storage doesn't support inserting empty values."
+			).to_compile_error();
+		}
+	}
+
+	// Otherwise implement some runtime tests
+	let mut tests = TokenStream::new();
+
+	for line in def.storage_lines.iter().filter(|l| !l.is_option && !l.is_generic) {
+		let test_name = syn::Ident::new(
+			&format!("test_default_value_for_{}_is_not_empty", line.name.to_string()),
+			line.name.span(),
+		);
+		let value_type = &line.value_type;
+		let default_value = line.default_value.as_ref().map(|d| quote!( #d ))
+			.unwrap_or_else(|| quote!( Default::default() ));
+		tests.extend(quote!{
+			#[test]
+			fn #test_name() {
+				use #scrate::codec::Encode;
+				let value: #value_type = #default_value;
+				assert!(!value.encode().is_empty());
+			}
+		});
+	}
+
+	quote!(
+		#[cfg(test)]
+		#[allow(non_snake_case)]
+		mod _decl_storage_assert_non_empty_value_tests {
+			use super::*;
+
+			#tests
+		}
+	)
+}

frame/support/procedural/src/storage/mod.rs

@@ -24,6 +24,7 @@ mod getters;
 mod metadata;
 mod instance_trait;
 mod genesis_config;
+mod empty_value_test;
 
 use quote::quote;
 use frame_support_procedural_tools::{
@@ -409,6 +410,7 @@ pub fn decl_storage_impl(input: proc_macro::TokenStream) -> proc_macro::TokenStr
 	let instance_trait = instance_trait::decl_and_impl(&scrate, &def_ext);
 	let genesis_config = genesis_config::genesis_config_and_build_storage(&scrate, &def_ext);
 	let storage_struct = storage_struct::decl_and_impl(&scrate, &def_ext);
+	let empty_value_test = empty_value_test::impl_empty_value_test(&scrate, &def_ext);
 
 	quote!(
 		use #scrate::{
@@ -425,5 +427,6 @@ pub fn decl_storage_impl(input: proc_macro::TokenStream) -> proc_macro::TokenStr
 		#instance_trait
 		#genesis_config
 		#storage_struct
+		#empty_value_test
 	).into()
 }

frame/support/src/storage/child.rs

@@ -77,19 +77,24 @@ pub fn get_or_else<T: Decode + Sized, F: FnOnce() -> T>(
 }
 
 /// Put `value` in storage under `key`.
+///
+/// # Warning
+///
+/// Put empty value is not supported (i.e value must encode to an non-empty slice).
 pub fn put<T: Encode>(
 	child_info: &ChildInfo,
 	key: &[u8],
 	value: &T,
 ) {
 	match child_info.child_type() {
-		ChildType::ParentKeyId => value.using_encoded(|slice|
+		ChildType::ParentKeyId => value.using_encoded(|slice| {
+			debug_assert!(!slice.is_empty(), "put empty slice in child storage is not supported");
 			sp_io::default_child_storage::set(
 				child_info.storage_key(),
 				key,
 				slice,
 			)
-		),
+		}),
 	}
 }
 
@@ -187,11 +192,16 @@ pub fn get_raw(
 }
 
 /// Put a raw byte slice into storage.
+///
+/// # Warning
+///
+/// Put empty value is not supported.
 pub fn put_raw(
 	child_info: &ChildInfo,
 	key: &[u8],
 	value: &[u8],
 ) {
+	debug_assert!(!value.is_empty(), "put_raw empty slice in child storage is not supported");
 	match child_info.child_type() {
 		ChildType::ParentKeyId => sp_io::default_child_storage::set(
 			child_info.storage_key(),

frame/support/src/storage/hashed.rs

@@ -66,6 +66,10 @@ where
 }
 
 /// Put `value` in storage under `key`.
+///
+/// # Warning
+///
+/// Put empty value is not supported (i.e value must encode to an non-empty slice).
 pub fn put<T, HashFn, R>(hash: &HashFn, key: &[u8], value: &T)
 where
 	T: Encode,
@@ -147,6 +151,10 @@ where
 }
 
 /// Put a raw byte slice into storage.
+///
+/// # Warning
+///
+/// Put empty value is not supported.
 pub fn put_raw<HashFn, R>(hash: &HashFn, key: &[u8], value: &[u8])
 where
 	HashFn: Fn(&[u8]) -> R,

frame/support/src/storage/unhashed.rs

@@ -50,8 +50,15 @@ pub fn get_or_else<T: Decode + Sized, F: FnOnce() -> T>(key: &[u8], default_valu
 }
 
 /// Put `value` in storage under `key`.
+///
+/// # Warning
+///
+/// Put empty value is not supported (i.e value must encode to an non-empty slice).
 pub fn put<T: Encode + ?Sized>(key: &[u8], value: &T) {
-	value.using_encoded(|slice| sp_io::storage::set(key, slice));
+	value.using_encoded(|slice| {
+		debug_assert!(!slice.is_empty(), "put empty slice in storage is not supported");
+		sp_io::storage::set(key, slice)
+	});
 }
 
 /// Remove `key` from storage, returning its value if it had an explicit entry or `None` otherwise.
@@ -103,9 +110,13 @@ pub fn get_raw(key: &[u8]) -> Option<Vec<u8>> {
 
 /// Put a raw byte slice into storage.
 ///
-/// **WARNING**: If you set the storage of the Substrate Wasm (`well_known_keys::CODE`),
-/// you should also call `frame_system::RuntimeUpgraded::put(true)` to trigger the
-/// `on_runtime_upgrade` logic.
+/// # Warning
+///
+/// * If you set the storage of the Substrate Wasm (`well_known_keys::CODE`),
+///   you should also call `frame_system::RuntimeUpgraded::put(true)` to trigger the
+///   `on_runtime_upgrade` logic.
+/// * Put empty value is not supported.
 pub fn put_raw(key: &[u8], value: &[u8]) {
+	debug_assert!(!value.is_empty(), "put_raw empty slice in storage is not supported");
 	sp_io::storage::set(key, value)
 }

frame/support/test/tests/decl_storage_ui/unit_type.rs

@@ -0,0 +1,33 @@
+// This file is part of Substrate.
+
+// Copyright (C) 2020 Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+pub trait Trait {
+	type Origin;
+	type BlockNumber: codec::Codec + codec::EncodeLike + Default + Clone;
+}
+
+frame_support::decl_module! {
+	pub struct Module<T: Trait> for enum Call where origin: T::Origin {}
+}
+
+frame_support::decl_storage!{
+	trait Store for Module<T: Trait> as FinalKeysNone {
+		pub Value: Option<()>;
+	}
+}
+
+fn main() {}

frame/support/test/tests/decl_storage_ui/unit_type.stderr

@@ -0,0 +1,5 @@
+error: `()` is not supported, because storage doesn't support inserting empty values.
+  --> $DIR/unit_type.rs:29:21
+   |
+29 |         pub Value: Option<()>;
+   |                           ^^

primitives/io/src/lib.rs

@@ -99,7 +99,14 @@ pub trait Storage {
 	}
 
 	/// Set `key` to `value` in the storage.
+	///
+	/// # Warning
+	///
+	/// Empty value is not supported.
 	fn set(&mut self, key: &[u8], value: &[u8]) {
+		if value.is_empty() {
+			log::error!(target: "runtime", "Empty value is inserted, not supported.");
+		}
 		self.set_storage(key.to_vec(), value.to_vec());
 	}
 
@@ -203,12 +210,19 @@ pub trait DefaultChildStorage {
 	/// Set a child storage value.
 	///
 	/// Set `key` to `value` in the child storage denoted by `storage_key`.
+	///
+	/// # Warning
+	///
+	/// Empty value is not supported.
 	fn set(
 		&mut self,
 		storage_key: &[u8],
 		key: &[u8],
 		value: &[u8],
 	) {
+		if value.is_empty() {
+			log::error!(target: "runtime", "Empty value is inserted in child, not supported.");
+		}
 		let child_info = ChildInfo::new_default(storage_key);
 		self.set_child_storage(&child_info, key.to_vec(), value.to_vec());
 	}