Allow pallet error enum variants to contain fields (#10242)

* Allow pallet errors to contain at most one field

* Update docs on pallet::error

* Reword documentation

* cargo fmt

* Introduce CompactPalletError trait and require #[pallet::error] fields to implement them

* cargo fmt

* Do not assume tuple variants

* Add CompactPalletError derive macro

* Check for error type compactness in construct_runtime

* cargo fmt

* Derive CompactPalletError instead of implementing it directly during macro expansion

* Implement CompactPalletError on OptionBool instead of Option<bool>

* Check for type idents instead of variant ident

* Add doc comments for ErrorCompactnessTest

* Add an trait implementation of ErrorCompactnessTest for ()

* Convert the error field of DispatchError to a 4-element byte array

* Add static check for pallet error size

* Rename to MAX_PALLET_ERROR_ENCODED_SIZE

* Remove ErrorCompactnessTest trait

* Remove check_compactness

* Return only the most significant byte when constructing a custom InvalidTransaction

* Rename CompactPalletError to PalletError

* Use counter to generate unique idents for assert macros

* Make declarative pallet macros compile with pallet error size checks

* Remove unused doc comment

* Try and fix build errors

* Fix build errors

* Add macro_use for some test modules

* Test fix

* Fix compilation errors

* Remove unneeded #[macro_use]

* Resolve import ambiguity

* Make path to pallet Error enum more specific

* Fix test expectation

* Disambiguate imports

* Fix test expectations

* Revert appending pallet module name to path

* Rename bags_list::list::Error to BagError

* Fixes

* Fixes

* Fixes

* Fix test expectations

* Fix test expectation

* Add more implementations for PalletError

* Lift the 1-field requirement for nested pallet errors

* Fix UI test expectation

* Remove PalletError impl for OptionBool

* Use saturating operations

* cargo fmt

* Delete obsolete test

* Fix test expectation

* Try and use assert macro in const context

* Pull out the pallet error size check macro

* Fix UI test for const assertion

* cargo fmt

* Apply clippy suggestion

* Fix doc comment

* Docs for create_tt_return_macro

* Ensure TryInto is imported in earlier Rust editions

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Fix up comments and names

* Implement PalletError for Never

* cargo fmt

* Don't compile example code

* Bump API version for block builder

* Factor in codec attributes while derving PalletError

* Rename module and fix unit test

* Add missing attribute

* Check API version and convert ApplyExtrinsicResult accordingly

* Rename BagError to ListError

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Use codec crate re-exported from frame support

* Add links to types mentioned in doc comments

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* cargo fmt

* cargo fmt

* Re-add attribute for hidden docs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

substrate/frame/support/src/dispatch.rs

@@ -2003,6 +2003,10 @@ macro_rules! decl_module {
 		pub type Pallet<$trait_instance $(, $instance $( = $module_default_instance)?)?>
 			= $mod_type<$trait_instance $(, $instance)?>;
 
+		$crate::__create_tt_macro! {
+			tt_error_token,
+		}
+
 		$crate::decl_module! {
 			@impl_on_initialize
 			{ $system }

substrate/frame/support/src/error.rs

@@ -85,7 +85,12 @@ macro_rules! decl_error {
 		}
 	) => {
 		$(#[$attr])*
-		#[derive($crate::scale_info::TypeInfo)]
+		#[derive(
+			$crate::codec::Encode,
+			$crate::codec::Decode,
+			$crate::scale_info::TypeInfo,
+			$crate::PalletError,
+		)]
 		#[scale_info(skip_type_params($generic $(, $inst_generic)?), capture_docs = "always")]
 		pub enum $error<$generic: $trait $(, $inst_generic: $instance)?>
 		$( where $( $where_ty: $where_bound ),* )?
@@ -114,17 +119,6 @@ macro_rules! decl_error {
 		impl<$generic: $trait $(, $inst_generic: $instance)?> $error<$generic $(, $inst_generic)?>
 		$( where $( $where_ty: $where_bound ),* )?
 		{
-			fn as_u8(&self) -> u8 {
-				$crate::decl_error! {
-					@GENERATE_AS_U8
-					self
-					$error
-					{}
-					0,
-					$( $name ),*
-				}
-			}
-
 			fn as_str(&self) -> &'static str {
 				match self {
 					Self::__Ignore(_, _) => unreachable!("`__Ignore` can never be constructed"),
@@ -149,47 +143,19 @@ macro_rules! decl_error {
 		$( where $( $where_ty: $where_bound ),* )?
 		{
 			fn from(err: $error<$generic $(, $inst_generic)?>) -> Self {
+				use $crate::codec::Encode;
 				let index = <$generic::PalletInfo as $crate::traits::PalletInfo>
 					::index::<$module<$generic $(, $inst_generic)?>>()
 					.expect("Every active module has an index in the runtime; qed") as u8;
+				let mut error = err.encode();
+				error.resize($crate::MAX_MODULE_ERROR_ENCODED_SIZE, 0);
 
 				$crate::sp_runtime::DispatchError::Module($crate::sp_runtime::ModuleError {
 					index,
-					error: err.as_u8(),
+					error: core::convert::TryInto::try_into(error).expect("encoded error is resized to be equal to the maximum encoded error size; qed"),
 					message: Some(err.as_str()),
 				})
 			}
 		}
 	};
-	(@GENERATE_AS_U8
-		$self:ident
-		$error:ident
-		{ $( $generated:tt )* }
-		$index:expr,
-		$name:ident
-		$( , $rest:ident )*
-	) => {
-		$crate::decl_error! {
-			@GENERATE_AS_U8
-			$self
-			$error
-			{
-				$( $generated )*
-				$error::$name => $index,
-			}
-			$index + 1,
-			$( $rest ),*
-		}
-	};
-	(@GENERATE_AS_U8
-		$self:ident
-		$error:ident
-		{ $( $generated:tt )* }
-		$index:expr,
-	) => {
-		match $self {
-			$error::__Ignore(_, _) => unreachable!("`__Ignore` can never be constructed"),
-			$( $generated )*
-		}
-	}
 }

substrate/frame/support/src/lib.rs

@@ -93,7 +93,9 @@ pub use self::{
 		StorageMap, StorageNMap, StoragePrefixedMap, StorageValue,
 	},
 };
-pub use sp_runtime::{self, print, traits::Printable, ConsensusEngineId};
+pub use sp_runtime::{
+	self, print, traits::Printable, ConsensusEngineId, MAX_MODULE_ERROR_ENCODED_SIZE,
+};
 
 use codec::{Decode, Encode};
 use scale_info::TypeInfo;
@@ -103,7 +105,7 @@ use sp_runtime::TypeId;
 pub const LOG_TARGET: &'static str = "runtime::frame-support";
 
 /// A type that cannot be instantiated.
-#[derive(Debug, PartialEq, Eq, Clone, TypeInfo)]
+#[derive(Encode, Decode, Debug, PartialEq, Eq, Clone, TypeInfo)]
 pub enum Never {}
 
 /// A pallet identifier. These are per pallet and should be stored in a registry somewhere.
@@ -598,11 +600,12 @@ pub fn debug(data: &impl sp_std::fmt::Debug) {
 
 #[doc(inline)]
 pub use frame_support_procedural::{
-	construct_runtime, decl_storage, match_and_insert, transactional, RuntimeDebugNoBound,
+	construct_runtime, decl_storage, match_and_insert, transactional, PalletError,
+	RuntimeDebugNoBound,
 };
 
 #[doc(hidden)]
-pub use frame_support_procedural::__generate_dummy_part_checker;
+pub use frame_support_procedural::{__create_tt_macro, __generate_dummy_part_checker};
 
 /// Derive [`Clone`] but do not bound any generic.
 ///
@@ -847,6 +850,32 @@ macro_rules! assert_ok {
 	};
 }
 
+/// Assert that the maximum encoding size does not exceed the value defined in
+/// [`MAX_MODULE_ERROR_ENCODED_SIZE`] during compilation.
+///
+/// This macro is intended to be used in conjunction with `tt_call!`.
+#[macro_export]
+macro_rules! assert_error_encoded_size {
+	{
+		path = [{ $($path:ident)::+ }]
+		runtime = [{ $runtime:ident }]
+		assert_message = [{ $assert_message:literal }]
+		error = [{ $error:ident }]
+	} => {
+		const _: () = assert!(
+			<
+				$($path::)+$error<$runtime> as $crate::traits::PalletError
+			>::MAX_ENCODED_SIZE <= $crate::MAX_MODULE_ERROR_ENCODED_SIZE,
+			$assert_message
+		);
+	};
+	{
+		path = [{ $($path:ident)::+ }]
+		runtime = [{ $runtime:ident }]
+		assert_message = [{ $assert_message:literal }]
+	} => {};
+}
+
 #[cfg(feature = "std")]
 #[doc(hidden)]
 pub use serde::{Deserialize, Serialize};
@@ -1375,6 +1404,7 @@ pub mod pallet_prelude {
 			TransactionTag, TransactionValidity, TransactionValidityError, UnknownTransaction,
 			ValidTransaction,
 		},
+		MAX_MODULE_ERROR_ENCODED_SIZE,
 	};
 	pub use sp_std::marker::PhantomData;
 }
@@ -1652,10 +1682,25 @@ pub mod pallet_prelude {
 /// pub enum Error<T> {
 /// 	/// $some_optional_doc
 /// 	$SomeFieldLessVariant,
+/// 	/// $some_more_optional_doc
+/// 	$SomeVariantWithOneField(FieldType),
 /// 	...
 /// }
 /// ```
-/// I.e. a regular rust enum named `Error`, with generic `T` and fieldless variants.
+/// I.e. a regular rust enum named `Error`, with generic `T` and fieldless or multiple-field
+/// variants.
+///
+/// Any field type in the enum variants must implement [`scale_info::TypeInfo`] in order to be
+/// properly used in the metadata, and its encoded size should be as small as possible,
+/// preferably 1 byte in size in order to reduce storage size. The error enum itself has an
+/// absolute maximum encoded size specified by [`MAX_MODULE_ERROR_ENCODED_SIZE`].
+///
+/// Field types in enum variants must also implement [`PalletError`](traits::PalletError),
+/// otherwise the pallet will fail to compile. Rust primitive types have already implemented
+/// the [`PalletError`](traits::PalletError) trait along with some commonly used stdlib types
+/// such as `Option` and `PhantomData`, and hence in most use cases, a manual implementation is
+/// not necessary and is discouraged.
+///
 /// The generic `T` mustn't bound anything and where clause is not allowed. But bounds and
 /// where clause shouldn't be needed for any usecase.
 ///

substrate/frame/support/src/traits.rs

@@ -46,6 +46,9 @@ pub use validation::{
 	ValidatorSetWithIdentification, VerifySeal,
 };
 
+mod error;
+pub use error::PalletError;
+
 mod filter;
 pub use filter::{ClearFilterGuard, FilterStack, FilterStackGuard, InstanceFilter, IntegrityTest};
 

substrate/frame/support/src/traits/error.rs

@@ -0,0 +1,95 @@
+// This file is part of Substrate.
+
+// Copyright (C) 2022 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.
+
+//! Traits for describing and constraining pallet error types.
+use codec::{Compact, Decode, Encode};
+use sp_std::marker::PhantomData;
+
+/// Trait indicating that the implementing type is going to be included as a field in a variant of
+/// the `#[pallet::error]` enum type.
+///
+/// ## Notes
+///
+/// The pallet error enum has a maximum encoded size as defined by
+/// [`frame_support::MAX_MODULE_ERROR_ENCODED_SIZE`]. If the pallet error type exceeds this size
+/// limit, a static assertion during compilation will fail. The compilation error will be in the
+/// format of `error[E0080]: evaluation of constant value failed` due to the usage of
+/// const assertions.
+pub trait PalletError: Encode + Decode {
+	/// The maximum encoded size for the implementing type.
+	///
+	/// This will be used to check whether the pallet error type is less than or equal to
+	/// [`frame_support::MAX_MODULE_ERROR_ENCODED_SIZE`], and if it is, a compilation error will be
+	/// thrown.
+	const MAX_ENCODED_SIZE: usize;
+}
+
+macro_rules! impl_for_types {
+	(size: $size:expr, $($typ:ty),+) => {
+		$(
+			impl PalletError for $typ {
+				const MAX_ENCODED_SIZE: usize = $size;
+			}
+		)+
+	};
+}
+
+impl_for_types!(size: 0, (), crate::Never);
+impl_for_types!(size: 1, u8, i8, bool);
+impl_for_types!(size: 2, u16, i16, Compact<u8>);
+impl_for_types!(size: 4, u32, i32, Compact<u16>);
+impl_for_types!(size: 5, Compact<u32>);
+impl_for_types!(size: 8, u64, i64);
+impl_for_types!(size: 9, Compact<u64>);
+// Contains a u64 for secs and u32 for nanos, hence 12 bytes
+impl_for_types!(size: 12, core::time::Duration);
+impl_for_types!(size: 16, u128, i128);
+impl_for_types!(size: 17, Compact<u128>);
+
+impl<T> PalletError for PhantomData<T> {
+	const MAX_ENCODED_SIZE: usize = 0;
+}
+
+impl<T: PalletError> PalletError for core::ops::Range<T> {
+	const MAX_ENCODED_SIZE: usize = T::MAX_ENCODED_SIZE.saturating_mul(2);
+}
+
+impl<T: PalletError, const N: usize> PalletError for [T; N] {
+	const MAX_ENCODED_SIZE: usize = T::MAX_ENCODED_SIZE.saturating_mul(N);
+}
+
+impl<T: PalletError> PalletError for Option<T> {
+	const MAX_ENCODED_SIZE: usize = T::MAX_ENCODED_SIZE.saturating_add(1);
+}
+
+impl<T: PalletError, E: PalletError> PalletError for Result<T, E> {
+	const MAX_ENCODED_SIZE: usize = if T::MAX_ENCODED_SIZE > E::MAX_ENCODED_SIZE {
+		T::MAX_ENCODED_SIZE
+	} else {
+		E::MAX_ENCODED_SIZE
+	}
+	.saturating_add(1);
+}
+
+#[impl_trait_for_tuples::impl_for_tuples(1, 18)]
+impl PalletError for Tuple {
+	const MAX_ENCODED_SIZE: usize = {
+		let mut size = 0_usize;
+		for_tuples!( #(size = size.saturating_add(Tuple::MAX_ENCODED_SIZE);)* );
+		size
+	};
+}