From a31637def8f00a7a980908581925211f1ec085fe Mon Sep 17 00:00:00 2001
From: Guillaume Thiolliere <gui.thiolliere@gmail.com>
Date: Thu, 1 Oct 2020 14:40:45 +0200
Subject: [PATCH] Fix executive test (#7243)

* fix executive test

* add unallowed unsigned test

* better to have also unsigned pre_dispatch check tested
---
 substrate/frame/executive/src/lib.rs          | 115 ++++++++++--------
 .../procedural/src/construct_runtime/mod.rs   |   6 +
 substrate/primitives/runtime/src/testing.rs   |  15 ++-
 3 files changed, 82 insertions(+), 54 deletions(-)

diff --git a/substrate/frame/executive/src/lib.rs b/substrate/frame/executive/src/lib.rs
index 7a5f39ccd8f..bbd077227a2 100644
--- a/substrate/frame/executive/src/lib.rs
+++ b/substrate/frame/executive/src/lib.rs
@@ -481,20 +481,25 @@ mod tests {
 	use sp_runtime::{
 		generic::Era, Perbill, DispatchError, testing::{Digest, Header, Block},
 		traits::{Header as HeaderT, BlakeTwo256, IdentityLookup},
-		transaction_validity::{InvalidTransaction, UnknownTransaction, TransactionValidityError},
+		transaction_validity::{
+			InvalidTransaction, ValidTransaction, TransactionValidityError, UnknownTransaction
+		},
 	};
 	use frame_support::{
-		impl_outer_event, impl_outer_origin, parameter_types, impl_outer_dispatch,
+		parameter_types,
 		weights::{Weight, RuntimeDbWeight, IdentityFee, WeightToFeePolynomial},
 		traits::{Currency, LockIdentifier, LockableCurrency, WithdrawReasons, WithdrawReason},
 	};
-	use frame_system::{self as system, Call as SystemCall, ChainContext, LastRuntimeUpgradeInfo};
+	use frame_system::{Call as SystemCall, ChainContext, LastRuntimeUpgradeInfo};
 	use pallet_balances::Call as BalancesCall;
 	use hex_literal::hex;
 	const TEST_KEY: &[u8] = &*b":test:key:";
 
 	mod custom {
 		use frame_support::weights::{Weight, DispatchClass};
+		use sp_runtime::transaction_validity::{
+			UnknownTransaction, TransactionSource, TransactionValidity
+		};
 
 		pub trait Trait: frame_system::Trait {}
 
@@ -514,6 +519,16 @@ mod tests {
 					let _ = frame_system::ensure_none(origin);
 				}
 
+				#[weight = 0]
+				fn allowed_unsigned(origin) {
+					let _ = frame_system::ensure_root(origin)?;
+				}
+
+				#[weight = 0]
+				fn unallowed_unsigned(origin) {
+					let _ = frame_system::ensure_root(origin)?;
+				}
+
 				// module hooks.
 				// one with block number arg and one without
 				fn on_initialize(n: T::BlockNumber) -> Weight {
@@ -531,33 +546,34 @@ mod tests {
 				}
 			}
 		}
-	}
 
-	type System = frame_system::Module<Runtime>;
-	type Balances = pallet_balances::Module<Runtime>;
-	type Custom = custom::Module<Runtime>;
+		impl<T: Trait> sp_runtime::traits::ValidateUnsigned for Module<T> {
+			type Call = Call<T>;
 
-	use pallet_balances as balances;
-
-	impl_outer_origin! {
-		pub enum Origin for Runtime { }
-	}
-
-	impl_outer_event!{
-		pub enum MetaEvent for Runtime {
-			system<T>,
-			balances<T>,
+			fn validate_unsigned(
+				_source: TransactionSource,
+				call: &Self::Call,
+			) -> TransactionValidity {
+				match call {
+					Call::allowed_unsigned(..) => Ok(Default::default()),
+					_ => UnknownTransaction::NoUnsignedValidator.into(),
+				}
+			}
 		}
 	}
-	impl_outer_dispatch! {
-		pub enum Call for Runtime where origin: Origin {
-			frame_system::System,
-			pallet_balances::Balances,
+
+	frame_support::construct_runtime!(
+		pub enum Runtime where
+			Block = TestBlock,
+			NodeBlock = TestBlock,
+			UncheckedExtrinsic = TestUncheckedExtrinsic
+		{
+			System: frame_system::{Module, Call, Config, Storage, Event<T>},
+			Balances: pallet_balances::{Module, Call, Storage, Config<T>, Event<T>},
+			Custom: custom::{Module, Call, ValidateUnsigned},
 		}
-	}
+	);
 
-	#[derive(Clone, Eq, PartialEq)]
-	pub struct Runtime;
 	parameter_types! {
 		pub const BlockHashCount: u64 = 250;
 		pub const MaximumBlockWeight: Weight = 1024;
@@ -581,7 +597,7 @@ mod tests {
 		type AccountId = u64;
 		type Lookup = IdentityLookup<u64>;
 		type Header = Header;
-		type Event = MetaEvent;
+		type Event = Event;
 		type BlockHashCount = BlockHashCount;
 		type MaximumBlockWeight = MaximumBlockWeight;
 		type DbWeight = DbWeight;
@@ -591,7 +607,7 @@ mod tests {
 		type AvailableBlockRatio = AvailableBlockRatio;
 		type MaximumBlockLength = MaximumBlockLength;
 		type Version = RuntimeVersion;
-		type PalletInfo = ();
+		type PalletInfo = PalletInfo;
 		type AccountData = pallet_balances::AccountData<Balance>;
 		type OnNewAccount = ();
 		type OnKilledAccount = ();
@@ -604,7 +620,7 @@ mod tests {
 	}
 	impl pallet_balances::Trait for Runtime {
 		type Balance = Balance;
-		type Event = MetaEvent;
+		type Event = Event;
 		type DustRemoval = ();
 		type ExistentialDeposit = ExistentialDeposit;
 		type AccountStore = System;
@@ -624,24 +640,6 @@ mod tests {
 	}
 	impl custom::Trait for Runtime {}
 
-	impl ValidateUnsigned for Runtime {
-		type Call = Call;
-
-		fn pre_dispatch(_call: &Self::Call) -> Result<(), TransactionValidityError> {
-			Ok(())
-		}
-
-		fn validate_unsigned(
-			_source: TransactionSource,
-			call: &Self::Call,
-		) -> TransactionValidity {
-			match call {
-				Call::Balances(BalancesCall::set_balance(_, _, _)) => Ok(Default::default()),
-				_ => UnknownTransaction::NoUnsignedValidator.into(),
-			}
-		}
-	}
-
 	pub struct RuntimeVersion;
 	impl frame_support::traits::Get<sp_version::RuntimeVersion> for RuntimeVersion {
 		fn get() -> sp_version::RuntimeVersion {
@@ -660,8 +658,14 @@ mod tests {
 		frame_system::CheckWeight<Runtime>,
 		pallet_transaction_payment::ChargeTransactionPayment<Runtime>,
 	);
-	type AllModules = (System, Balances, Custom);
 	type TestXt = sp_runtime::testing::TestXt<Call, SignedExtra>;
+	type TestBlock = Block<TestXt>;
+	type TestUncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic<
+		<Runtime as frame_system::Trait>::AccountId,
+		<Runtime as frame_system::Trait>::Call,
+		(),
+		SignedExtra,
+	>;
 
 	// Will contain `true` when the custom runtime logic was called.
 	const CUSTOM_ON_RUNTIME_KEY: &[u8] = &*b":custom:on_runtime";
@@ -895,15 +899,26 @@ mod tests {
 
 	#[test]
 	fn validate_unsigned() {
-		let xt = TestXt::new(Call::Balances(BalancesCall::set_balance(33, 69, 69)), None);
+		let valid = TestXt::new(Call::Custom(custom::Call::allowed_unsigned()), None);
+		let invalid = TestXt::new(Call::Custom(custom::Call::unallowed_unsigned()), None);
 		let mut t = new_test_ext(1);
 
+		let mut default_with_prio_3 = ValidTransaction::default();
+		default_with_prio_3.priority = 3;
 		t.execute_with(|| {
 			assert_eq!(
-				Executive::validate_transaction(TransactionSource::InBlock, xt.clone()),
-				Ok(Default::default()),
+				Executive::validate_transaction(TransactionSource::InBlock, valid.clone()),
+				Ok(default_with_prio_3),
+			);
+			assert_eq!(
+				Executive::validate_transaction(TransactionSource::InBlock, invalid.clone()),
+				Err(TransactionValidityError::Unknown(UnknownTransaction::NoUnsignedValidator)),
+			);
+			assert_eq!(Executive::apply_extrinsic(valid), Ok(Err(DispatchError::BadOrigin)));
+			assert_eq!(
+				Executive::apply_extrinsic(invalid),
+				Err(TransactionValidityError::Unknown(UnknownTransaction::NoUnsignedValidator))
 			);
-			assert_eq!(Executive::apply_extrinsic(xt), Ok(Err(DispatchError::BadOrigin)));
 		});
 	}
 
diff --git a/substrate/frame/support/procedural/src/construct_runtime/mod.rs b/substrate/frame/support/procedural/src/construct_runtime/mod.rs
index c51582fdd05..f355593defb 100644
--- a/substrate/frame/support/procedural/src/construct_runtime/mod.rs
+++ b/substrate/frame/support/procedural/src/construct_runtime/mod.rs
@@ -169,6 +169,12 @@ fn construct_runtime_parsed(definition: RuntimeDefinition) -> Result<TokenStream
 	let res = quote!(
 		#scrate_decl
 
+		// Prevent UncheckedExtrinsic to print unused warning.
+		const _: () = {
+			#[allow(unused)]
+			type __hidden_use_of_unchecked_extrinsic = #unchecked_extrinsic;
+		};
+
 		#[derive(Clone, Copy, PartialEq, Eq, #scrate::sp_runtime::RuntimeDebug)]
 		pub struct #name;
 		impl #scrate::sp_runtime::traits::GetNodeBlockType for #name {
diff --git a/substrate/primitives/runtime/src/testing.rs b/substrate/primitives/runtime/src/testing.rs
index eefb36ae827..97e128f363c 100644
--- a/substrate/primitives/runtime/src/testing.rs
+++ b/substrate/primitives/runtime/src/testing.rs
@@ -313,11 +313,17 @@ impl<Origin, Call, Extra> Applyable for TestXt<Call, Extra> where
 	/// Checks to see if this is a valid *transaction*. It returns information on it if so.
 	fn validate<U: ValidateUnsigned<Call=Self::Call>>(
 		&self,
-		_source: TransactionSource,
-		_info: &DispatchInfoOf<Self::Call>,
-		_len: usize,
+		source: TransactionSource,
+		info: &DispatchInfoOf<Self::Call>,
+		len: usize,
 	) -> TransactionValidity {
-		Ok(Default::default())
+		if let Some((ref id, ref extra)) = self.signature {
+			Extra::validate(extra, id, &self.call, info, len)
+		} else {
+			let valid = Extra::validate_unsigned(&self.call, info, len)?;
+			let unsigned_validation = U::validate_unsigned(source, &self.call)?;
+			Ok(valid.combine_with(unsigned_validation))
+		}
 	}
 
 	/// Executes all necessary logic needed prior to dispatch and deconstructs into function call,
@@ -332,6 +338,7 @@ impl<Origin, Call, Extra> Applyable for TestXt<Call, Extra> where
 			Some(who)
 		} else {
 			Extra::pre_dispatch_unsigned(&self.call, info, len)?;
+			U::pre_dispatch(&self.call)?;
 			None
 		};
 
-- 
GitLab