Introduce CheckNonZeroSender (paritytech#10413)

* Introduce CheckNonZeroSender

* Missing file

* Formatting

* Fixes

* Formatting

* some fixes to compile

* Update frame/system/src/extensions/check_non_zero_sender.rs

Co-authored-by: Bastian Köcher <[email protected]>

* Fixes

* Fixes

* another fix

* Formatting

Co-authored-by: Shawn Tabrizi <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>

bin/node/cli/src/service.rs

@@ -83,6 +83,7 @@ pub fn create_extrinsic(
 		.unwrap_or(2) as u64;
 	let tip = 0;
 	let extra: node_runtime::SignedExtra = (
+		frame_system::CheckNonZeroSender::<node_runtime::Runtime>::new(),
 		frame_system::CheckSpecVersion::<node_runtime::Runtime>::new(),
 		frame_system::CheckTxVersion::<node_runtime::Runtime>::new(),
 		frame_system::CheckGenesis::<node_runtime::Runtime>::new(),
@@ -99,6 +100,7 @@ pub fn create_extrinsic(
 		function.clone(),
 		extra.clone(),
 		(
+			(),
 			node_runtime::VERSION.spec_version,
 			node_runtime::VERSION.transaction_version,
 			genesis_hash,
@@ -719,6 +721,7 @@ mod tests {
 				let function =
 					Call::Balances(BalancesCall::transfer { dest: to.into(), value: amount });
 
+				let check_non_zero_sender = frame_system::CheckNonZeroSender::new();
 				let check_spec_version = frame_system::CheckSpecVersion::new();
 				let check_tx_version = frame_system::CheckTxVersion::new();
 				let check_genesis = frame_system::CheckGenesis::new();
@@ -727,6 +730,7 @@ mod tests {
 				let check_weight = frame_system::CheckWeight::new();
 				let tx_payment = pallet_asset_tx_payment::ChargeAssetTxPayment::from(0, None);
 				let extra = (
+					check_non_zero_sender,
 					check_spec_version,
 					check_tx_version,
 					check_genesis,
@@ -738,7 +742,7 @@ mod tests {
 				let raw_payload = SignedPayload::from_raw(
 					function,
 					extra,
-					(spec_version, transaction_version, genesis_hash, genesis_hash, (), (), ()),
+					((), spec_version, transaction_version, genesis_hash, genesis_hash, (), (), ()),
 				);
 				let signature = raw_payload.using_encoded(|payload| signer.sign(payload));
 				let (function, extra, _) = raw_payload.deconstruct();

bin/node/executor/tests/submit_transaction.rs

@@ -146,7 +146,7 @@ fn should_submit_signed_twice_from_the_same_account() {
 		let s = state.read();
 		fn nonce(tx: UncheckedExtrinsic) -> frame_system::CheckNonce<Runtime> {
 			let extra = tx.signature.unwrap().2;
-			extra.4
+			extra.5
 		}
 		let nonce1 = nonce(UncheckedExtrinsic::decode(&mut &*s.transactions[0]).unwrap());
 		let nonce2 = nonce(UncheckedExtrinsic::decode(&mut &*s.transactions[1]).unwrap());
@@ -195,7 +195,7 @@ fn should_submit_signed_twice_from_all_accounts() {
 		let s = state.read();
 		fn nonce(tx: UncheckedExtrinsic) -> frame_system::CheckNonce<Runtime> {
 			let extra = tx.signature.unwrap().2;
-			extra.4
+			extra.5
 		}
 		let nonce1 = nonce(UncheckedExtrinsic::decode(&mut &*s.transactions[0]).unwrap());
 		let nonce2 = nonce(UncheckedExtrinsic::decode(&mut &*s.transactions[1]).unwrap());

bin/node/runtime/src/lib.rs

@@ -984,6 +984,7 @@ where
 			.saturating_sub(1);
 		let era = Era::mortal(period, current_block);
 		let extra = (
+			frame_system::CheckNonZeroSender::<Runtime>::new(),
 			frame_system::CheckSpecVersion::<Runtime>::new(),
 			frame_system::CheckTxVersion::<Runtime>::new(),
 			frame_system::CheckGenesis::<Runtime>::new(),
@@ -1334,6 +1335,7 @@ pub type BlockId = generic::BlockId<Block>;
 ///
 /// [`sign`]: <../../testing/src/keyring.rs.html>
 pub type SignedExtra = (
+	frame_system::CheckNonZeroSender<Runtime>,
 	frame_system::CheckSpecVersion<Runtime>,
 	frame_system::CheckTxVersion<Runtime>,
 	frame_system::CheckGenesis<Runtime>,

bin/node/test-runner-example/src/lib.rs

@@ -69,6 +69,7 @@ impl ChainInfo for NodeTemplateChainInfo {
 		from: <Self::Runtime as frame_system::Config>::AccountId,
 	) -> Self::SignedExtras {
 		(
+			frame_system::CheckNonZeroSender::<Self::Runtime>::new(),
 			frame_system::CheckSpecVersion::<Self::Runtime>::new(),
 			frame_system::CheckTxVersion::<Self::Runtime>::new(),
 			frame_system::CheckGenesis::<Self::Runtime>::new(),

bin/node/testing/src/keyring.rs

@@ -70,6 +70,7 @@ pub fn to_session_keys(
 /// Returns transaction extra.
 pub fn signed_extra(nonce: Index, extra_fee: Balance) -> SignedExtra {
 	(
+		frame_system::CheckNonZeroSender::new(),
 		frame_system::CheckSpecVersion::new(),
 		frame_system::CheckTxVersion::new(),
 		frame_system::CheckGenesis::new(),

frame/system/src/extensions/check_non_zero_sender.rs

@@ -0,0 +1,100 @@
+// This file is part of Substrate.
+
+// Copyright (C) 2017-2021 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.
+
+use crate::Config;
+use codec::{Decode, Encode};
+use frame_support::weights::DispatchInfo;
+use scale_info::TypeInfo;
+use sp_runtime::{
+	traits::{DispatchInfoOf, Dispatchable, SignedExtension},
+	transaction_validity::{
+		InvalidTransaction, TransactionValidity, TransactionValidityError, ValidTransaction,
+	},
+};
+use sp_std::{marker::PhantomData, prelude::*};
+
+/// Check to ensure that the sender is not the zero address.
+#[derive(Encode, Decode, Clone, Eq, PartialEq, TypeInfo)]
+#[scale_info(skip_type_params(T))]
+pub struct CheckNonZeroSender<T: Config + Send + Sync>(PhantomData<T>);
+
+impl<T: Config + Send + Sync> sp_std::fmt::Debug for CheckNonZeroSender<T> {
+	#[cfg(feature = "std")]
+	fn fmt(&self, f: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result {
+		write!(f, "CheckNonZeroSender")
+	}
+
+	#[cfg(not(feature = "std"))]
+	fn fmt(&self, _: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result {
+		Ok(())
+	}
+}
+
+impl<T: Config + Send + Sync> CheckNonZeroSender<T> {
+	/// Create new `SignedExtension` to check runtime version.
+	pub fn new() -> Self {
+		Self(sp_std::marker::PhantomData)
+	}
+}
+
+impl<T: Config + Send + Sync> SignedExtension for CheckNonZeroSender<T>
+where
+	T::Call: Dispatchable<Info = DispatchInfo>,
+{
+	type AccountId = T::AccountId;
+	type Call = T::Call;
+	type AdditionalSigned = ();
+	type Pre = ();
+	const IDENTIFIER: &'static str = "CheckNonZeroSender";
+
+	fn additional_signed(&self) -> sp_std::result::Result<(), TransactionValidityError> {
+		Ok(())
+	}
+
+	fn validate(
+		&self,
+		who: &Self::AccountId,
+		_call: &Self::Call,
+		_info: &DispatchInfoOf<Self::Call>,
+		_len: usize,
+	) -> TransactionValidity {
+		if who.using_encoded(|d| d.into_iter().all(|x| *x == 0)) {
+			return Err(TransactionValidityError::Invalid(InvalidTransaction::BadSigner))
+		}
+		Ok(ValidTransaction::default())
+	}
+}
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+	use crate::mock::{new_test_ext, Test, CALL};
+	use frame_support::{assert_noop, assert_ok};
+
+	#[test]
+	fn zero_account_ban_works() {
+		new_test_ext().execute_with(|| {
+			let info = DispatchInfo::default();
+			let len = 0_usize;
+			assert_noop!(
+				CheckNonZeroSender::<Test>::new().validate(&0, CALL, &info, len),
+				InvalidTransaction::BadSigner
+			);
+			assert_ok!(CheckNonZeroSender::<Test>::new().validate(&1, CALL, &info, len));
+		})
+	}
+}

frame/system/src/extensions/mod.rs

@@ -17,6 +17,7 @@
 
 pub mod check_genesis;
 pub mod check_mortality;
+pub mod check_non_zero_sender;
 pub mod check_nonce;
 pub mod check_spec_version;
 pub mod check_tx_version;

frame/system/src/lib.rs

@@ -115,7 +115,8 @@ mod tests;
 pub mod weights;
 
 pub use extensions::{
-	check_genesis::CheckGenesis, check_mortality::CheckMortality, check_nonce::CheckNonce,
+	check_genesis::CheckGenesis, check_mortality::CheckMortality,
+	check_non_zero_sender::CheckNonZeroSender, check_nonce::CheckNonce,
 	check_spec_version::CheckSpecVersion, check_tx_version::CheckTxVersion,
 	check_weight::CheckWeight,
 };

primitives/runtime/src/transaction_validity.rs

@@ -79,6 +79,8 @@ pub enum InvalidTransaction {
 	/// A transaction with a mandatory dispatch. This is invalid; only inherent extrinsics are
 	/// allowed to have mandatory dispatches.
 	MandatoryDispatch,
+	/// The sending address is disabled or known to be invalid.
+	BadSigner,
 }
 
 impl InvalidTransaction {
@@ -109,6 +111,7 @@ impl From<InvalidTransaction> for &'static str {
 			InvalidTransaction::MandatoryDispatch =>
 				"Transaction dispatch is mandatory; transactions may not have mandatory dispatches.",
 			InvalidTransaction::Custom(_) => "InvalidTransaction custom error",
+			InvalidTransaction::BadSigner => "Invalid signing address",
 		}
 	}
 }

test-utils/test-runner/src/lib.rs

@@ -106,6 +106,7 @@
 //! 		let nonce = frame_system::Pallet::<Self::Runtime>::account_nonce(from);
 //!
 //! 		(
+//! 			frame_system::CheckNonZeroSender::<Self::Runtime>::new(),
 //! 			frame_system::CheckSpecVersion::<Self::Runtime>::new(),
 //! 			frame_system::CheckTxVersion::<Self::Runtime>::new(),
 //! 			frame_system::CheckGenesis::<Self::Runtime>::new(),