diff --git a/substrate/frame/election-provider-multi-phase/src/lib.rs b/substrate/frame/election-provider-multi-phase/src/lib.rs index 0a620e6c12..a5ed593e1a 100644 --- a/substrate/frame/election-provider-multi-phase/src/lib.rs +++ b/substrate/frame/election-provider-multi-phase/src/lib.rs @@ -255,7 +255,7 @@ use sp_runtime::{ InvalidTransaction, TransactionPriority, TransactionSource, TransactionValidity, TransactionValidityError, ValidTransaction, }, - DispatchError, PerThing, Perbill, RuntimeDebug, SaturatedConversion, + DispatchError, ModuleError, PerThing, Perbill, RuntimeDebug, SaturatedConversion, }; use sp_std::prelude::*; @@ -1579,7 +1579,7 @@ impl ElectionProvider for Pallet { /// number. pub fn dispatch_error_to_invalid(error: DispatchError) -> InvalidTransaction { let error_number = match error { - DispatchError::Module { error, .. } => error, + DispatchError::Module(ModuleError { error, .. }) => error, _ => 0, }; InvalidTransaction::Custom(error_number) diff --git a/substrate/frame/election-provider-multi-phase/src/unsigned.rs b/substrate/frame/election-provider-multi-phase/src/unsigned.rs index 936993b41f..196147f8a4 100644 --- a/substrate/frame/election-provider-multi-phase/src/unsigned.rs +++ b/substrate/frame/election-provider-multi-phase/src/unsigned.rs @@ -752,7 +752,7 @@ mod tests { use sp_runtime::{ offchain::storage_lock::{BlockAndTime, StorageLock}, traits::ValidateUnsigned, - PerU16, + ModuleError, PerU16, }; type Assignment = crate::unsigned::Assignment; @@ -922,8 +922,8 @@ mod tests { #[test] #[should_panic(expected = "Invalid unsigned submission must produce invalid block and \ deprive validator from their authoring reward.: \ - Module { index: 2, error: 1, message: \ - Some(\"PreDispatchWrongWinnerCount\") }")] + Module(ModuleError { index: 2, error: 1, message: \ + Some(\"PreDispatchWrongWinnerCount\") })")] fn unfeasible_solution_panics() { ExtBuilder::default().build_and_execute(|| { roll_to(25); @@ -1033,11 +1033,11 @@ mod tests { assert_eq!( MultiPhase::mine_check_save_submit().unwrap_err(), - MinerError::PreDispatchChecksFailed(DispatchError::Module { + MinerError::PreDispatchChecksFailed(DispatchError::Module(ModuleError { index: 2, error: 1, message: Some("PreDispatchWrongWinnerCount"), - }), + })), ); }) } diff --git a/substrate/frame/elections-phragmen/src/lib.rs b/substrate/frame/elections-phragmen/src/lib.rs index bc6a81125e..4758c793cf 100644 --- a/substrate/frame/elections-phragmen/src/lib.rs +++ b/substrate/frame/elections-phragmen/src/lib.rs @@ -1132,7 +1132,7 @@ mod tests { use sp_runtime::{ testing::Header, traits::{BlakeTwo256, IdentityLookup}, - BuildStorage, + BuildStorage, ModuleError, }; use substrate_test_utils::assert_eq_uvec; @@ -2514,7 +2514,7 @@ mod tests { let unwrapped_error = Elections::remove_member(Origin::root(), 4, true).unwrap_err(); assert!(matches!( unwrapped_error.error, - DispatchError::Module { message: Some("InvalidReplacement"), .. } + DispatchError::Module(ModuleError { message: Some("InvalidReplacement"), .. }) )); assert!(unwrapped_error.post_info.actual_weight.is_some()); }); @@ -2537,7 +2537,7 @@ mod tests { let unwrapped_error = Elections::remove_member(Origin::root(), 4, false).unwrap_err(); assert!(matches!( unwrapped_error.error, - DispatchError::Module { message: Some("InvalidReplacement"), .. } + DispatchError::Module(ModuleError { message: Some("InvalidReplacement"), .. }) )); assert!(unwrapped_error.post_info.actual_weight.is_some()); }); diff --git a/substrate/frame/support/procedural/src/pallet/expand/error.rs b/substrate/frame/support/procedural/src/pallet/expand/error.rs index 4cf572c179..9e2b801083 100644 --- a/substrate/frame/support/procedural/src/pallet/expand/error.rs +++ b/substrate/frame/support/procedural/src/pallet/expand/error.rs @@ -124,11 +124,11 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { >::index::>() .expect("Every active module has an index in the runtime; qed") as u8; - #frame_support::sp_runtime::DispatchError::Module { + #frame_support::sp_runtime::DispatchError::Module(#frame_support::sp_runtime::ModuleError { index, error: err.as_u8(), message: Some(err.as_str()), - } + }) } } ) diff --git a/substrate/frame/support/src/error.rs b/substrate/frame/support/src/error.rs index abac4a5326..4880bba5c5 100644 --- a/substrate/frame/support/src/error.rs +++ b/substrate/frame/support/src/error.rs @@ -153,11 +153,11 @@ macro_rules! decl_error { ::index::<$module<$generic $(, $inst_generic)?>>() .expect("Every active module has an index in the runtime; qed") as u8; - $crate::sp_runtime::DispatchError::Module { + $crate::sp_runtime::DispatchError::Module($crate::sp_runtime::ModuleError { index, error: err.as_u8(), message: Some(err.as_str()), - } + }) } } }; diff --git a/substrate/frame/support/test/tests/construct_runtime.rs b/substrate/frame/support/test/tests/construct_runtime.rs index 267d560edf..b3f8feb8aa 100644 --- a/substrate/frame/support/test/tests/construct_runtime.rs +++ b/substrate/frame/support/test/tests/construct_runtime.rs @@ -27,7 +27,7 @@ use sp_core::{sr25519, H256}; use sp_runtime::{ generic, traits::{BlakeTwo256, Verify}, - DispatchError, + DispatchError, ModuleError, }; use sp_std::cell::RefCell; @@ -363,47 +363,47 @@ mod origin_test { fn check_modules_error_type() { assert_eq!( Module1_1::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 31, error: 0, message: Some("Something") }), + Err(DispatchError::Module(ModuleError { index: 31, error: 0, message: Some("Something") })), ); assert_eq!( Module2::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 32, error: 0, message: Some("Something") }), + Err(DispatchError::Module(ModuleError { index: 32, error: 0, message: Some("Something") })), ); assert_eq!( Module1_2::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 33, error: 0, message: Some("Something") }), + Err(DispatchError::Module(ModuleError { index: 33, error: 0, message: Some("Something") })), ); assert_eq!( NestedModule3::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 34, error: 0, message: Some("Something") }), + Err(DispatchError::Module(ModuleError { index: 34, error: 0, message: Some("Something") })), ); assert_eq!( Module1_3::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 6, error: 0, message: Some("Something") }), + Err(DispatchError::Module(ModuleError { index: 6, error: 0, message: Some("Something") })), ); assert_eq!( Module1_4::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 3, error: 0, message: Some("Something") }), + Err(DispatchError::Module(ModuleError { index: 3, error: 0, message: Some("Something") })), ); assert_eq!( Module1_5::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 4, error: 0, message: Some("Something") }), + Err(DispatchError::Module(ModuleError { index: 4, error: 0, message: Some("Something") })), ); assert_eq!( Module1_6::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 1, error: 0, message: Some("Something") }), + Err(DispatchError::Module(ModuleError { index: 1, error: 0, message: Some("Something") })), ); assert_eq!( Module1_7::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 2, error: 0, message: Some("Something") }), + Err(DispatchError::Module(ModuleError { index: 2, error: 0, message: Some("Something") })), ); assert_eq!( Module1_8::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 12, error: 0, message: Some("Something") }), + Err(DispatchError::Module(ModuleError { index: 12, error: 0, message: Some("Something") })), ); assert_eq!( Module1_9::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 13, error: 0, message: Some("Something") }), + Err(DispatchError::Module(ModuleError { index: 13, error: 0, message: Some("Something") })), ); } diff --git a/substrate/frame/support/test/tests/pallet.rs b/substrate/frame/support/test/tests/pallet.rs index cf88255608..451cb2e7b8 100644 --- a/substrate/frame/support/test/tests/pallet.rs +++ b/substrate/frame/support/test/tests/pallet.rs @@ -29,7 +29,7 @@ use sp_io::{ hashing::{blake2_128, twox_128, twox_64}, TestExternalities, }; -use sp_runtime::DispatchError; +use sp_runtime::{DispatchError, ModuleError}; pub struct SomeType1; impl From for u64 { @@ -654,7 +654,11 @@ fn error_expand() { ); assert_eq!( DispatchError::from(pallet::Error::::InsufficientProposersBalance), - DispatchError::Module { index: 1, error: 0, message: Some("InsufficientProposersBalance") }, + DispatchError::Module(ModuleError { + index: 1, + error: 0, + message: Some("InsufficientProposersBalance") + }), ); } diff --git a/substrate/frame/support/test/tests/pallet_instance.rs b/substrate/frame/support/test/tests/pallet_instance.rs index 4a8636919b..30b9bcda88 100644 --- a/substrate/frame/support/test/tests/pallet_instance.rs +++ b/substrate/frame/support/test/tests/pallet_instance.rs @@ -25,7 +25,7 @@ use sp_io::{ hashing::{blake2_128, twox_128, twox_64}, TestExternalities, }; -use sp_runtime::DispatchError; +use sp_runtime::{DispatchError, ModuleError}; #[frame_support::pallet] pub mod pallet { @@ -341,7 +341,11 @@ fn error_expand() { ); assert_eq!( DispatchError::from(pallet::Error::::InsufficientProposersBalance), - DispatchError::Module { index: 1, error: 0, message: Some("InsufficientProposersBalance") }, + DispatchError::Module(ModuleError { + index: 1, + error: 0, + message: Some("InsufficientProposersBalance") + }), ); assert_eq!( @@ -358,7 +362,11 @@ fn error_expand() { DispatchError::from( pallet::Error::::InsufficientProposersBalance ), - DispatchError::Module { index: 2, error: 0, message: Some("InsufficientProposersBalance") }, + DispatchError::Module(ModuleError { + index: 2, + error: 0, + message: Some("InsufficientProposersBalance") + }), ); } diff --git a/substrate/primitives/runtime/src/lib.rs b/substrate/primitives/runtime/src/lib.rs index 84817a6be5..6d32d2322c 100644 --- a/substrate/primitives/runtime/src/lib.rs +++ b/substrate/primitives/runtime/src/lib.rs @@ -461,9 +461,29 @@ pub type DispatchResult = sp_std::result::Result<(), DispatchError>; /// about the `Dispatchable` that is only known post dispatch. pub type DispatchResultWithInfo = sp_std::result::Result>; -/// Reason why a dispatch call failed. +/// Reason why a pallet call failed. #[derive(Eq, Clone, Copy, Encode, Decode, Debug, TypeInfo)] #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] +pub struct ModuleError { + /// Module index, matching the metadata module index. + pub index: u8, + /// Module specific error value. + pub error: u8, + /// Optional error message. + #[codec(skip)] + #[cfg_attr(feature = "std", serde(skip_deserializing))] + pub message: Option<&'static str>, +} + +impl PartialEq for ModuleError { + fn eq(&self, other: &Self) -> bool { + (self.index == other.index) && (self.error == other.error) + } +} + +/// Reason why a dispatch call failed. +#[derive(Eq, Clone, Copy, Encode, Decode, Debug, TypeInfo, PartialEq)] +#[cfg_attr(feature = "std", derive(Serialize, Deserialize))] pub enum DispatchError { /// Some error occurred. Other( @@ -476,16 +496,7 @@ pub enum DispatchError { /// A bad origin. BadOrigin, /// A custom error in a module. - Module { - /// Module index, matching the metadata module index. - index: u8, - /// Module specific error value. - error: u8, - /// Optional error message. - #[codec(skip)] - #[cfg_attr(feature = "std", serde(skip_deserializing))] - message: Option<&'static str>, - }, + Module(ModuleError), /// At least one consumer is remaining so the account cannot be destroyed. ConsumerRemaining, /// There are no providers so the account cannot be created. @@ -515,8 +526,8 @@ impl DispatchError { /// Return the same error but without the attached message. pub fn stripped(self) -> Self { match self { - DispatchError::Module { index, error, message: Some(_) } => - DispatchError::Module { index, error, message: None }, + DispatchError::Module(ModuleError { index, error, message: Some(_) }) => + DispatchError::Module(ModuleError { index, error, message: None }), m => m, } } @@ -624,7 +635,8 @@ impl From for &'static str { DispatchError::Other(msg) => msg, DispatchError::CannotLookup => "Cannot lookup", DispatchError::BadOrigin => "Bad origin", - DispatchError::Module { message, .. } => message.unwrap_or("Unknown module error"), + DispatchError::Module(ModuleError { message, .. }) => + message.unwrap_or("Unknown module error"), DispatchError::ConsumerRemaining => "Consumer remaining", DispatchError::NoProviders => "No providers", DispatchError::TooManyConsumers => "Too many consumers", @@ -650,7 +662,7 @@ impl traits::Printable for DispatchError { Self::Other(err) => err.print(), Self::CannotLookup => "Cannot lookup".print(), Self::BadOrigin => "Bad origin".print(), - Self::Module { index, error, message } => { + Self::Module(ModuleError { index, error, message }) => { index.print(); error.print(); if let Some(msg) = message { @@ -683,30 +695,6 @@ where } } -impl PartialEq for DispatchError { - fn eq(&self, other: &Self) -> bool { - use DispatchError::*; - - match (self, other) { - (CannotLookup, CannotLookup) | - (BadOrigin, BadOrigin) | - (ConsumerRemaining, ConsumerRemaining) | - (NoProviders, NoProviders) => true, - - (Token(l), Token(r)) => l == r, - (Other(l), Other(r)) => l == r, - (Arithmetic(l), Arithmetic(r)) => l == r, - - ( - Module { index: index_l, error: error_l, .. }, - Module { index: index_r, error: error_r, .. }, - ) => (index_l == index_r) && (error_l == error_r), - - _ => false, - } - } -} - /// This type specifies the outcome of dispatching a call to a module. /// /// In case of failure an error specific to the module is returned. @@ -932,11 +920,18 @@ mod tests { #[test] fn dispatch_error_encoding() { - let error = DispatchError::Module { index: 1, error: 2, message: Some("error message") }; + let error = DispatchError::Module(ModuleError { + index: 1, + error: 2, + message: Some("error message"), + }); let encoded = error.encode(); let decoded = DispatchError::decode(&mut &encoded[..]).unwrap(); assert_eq!(encoded, vec![3, 1, 2]); - assert_eq!(decoded, DispatchError::Module { index: 1, error: 2, message: None }); + assert_eq!( + decoded, + DispatchError::Module(ModuleError { index: 1, error: 2, message: None }) + ); } #[test] @@ -948,9 +943,9 @@ mod tests { Other("bar"), CannotLookup, BadOrigin, - Module { index: 1, error: 1, message: None }, - Module { index: 1, error: 2, message: None }, - Module { index: 2, error: 1, message: None }, + Module(ModuleError { index: 1, error: 1, message: None }), + Module(ModuleError { index: 1, error: 2, message: None }), + Module(ModuleError { index: 2, error: 1, message: None }), ConsumerRemaining, NoProviders, Token(TokenError::NoFunds), @@ -975,8 +970,8 @@ mod tests { // Ignores `message` field in `Module` variant. assert_eq!( - Module { index: 1, error: 1, message: Some("foo") }, - Module { index: 1, error: 1, message: None }, + Module(ModuleError { index: 1, error: 1, message: Some("foo") }), + Module(ModuleError { index: 1, error: 1, message: None }), ); }