From 96dc79cedfd60892e66542119054ba9309e6cb4f Mon Sep 17 00:00:00 2001 From: Gavin Wood Date: Wed, 8 Dec 2021 11:28:03 +0100 Subject: [PATCH] Introduce CheckNonZeroSender (#10413) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * Fixes * Fixes * another fix * Formatting Co-authored-by: Shawn Tabrizi Co-authored-by: Bastian Köcher --- bin/node/cli/src/service.rs | 6 +- bin/node/executor/tests/submit_transaction.rs | 4 +- bin/node/runtime/src/lib.rs | 2 + bin/node/test-runner-example/src/lib.rs | 1 + bin/node/testing/src/keyring.rs | 1 + .../src/extensions/check_non_zero_sender.rs | 100 ++++++++++++++++++ frame/system/src/extensions/mod.rs | 1 + frame/system/src/lib.rs | 3 +- .../runtime/src/transaction_validity.rs | 3 + test-utils/test-runner/src/lib.rs | 1 + 10 files changed, 118 insertions(+), 4 deletions(-) create mode 100644 frame/system/src/extensions/check_non_zero_sender.rs diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index fec91a9b67cc4..1dfce9331b752 100644 --- a/bin/node/cli/src/service.rs +++ b/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::::new(), frame_system::CheckSpecVersion::::new(), frame_system::CheckTxVersion::::new(), frame_system::CheckGenesis::::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(); diff --git a/bin/node/executor/tests/submit_transaction.rs b/bin/node/executor/tests/submit_transaction.rs index f047c6a44a667..4f0f8061641f8 100644 --- a/bin/node/executor/tests/submit_transaction.rs +++ b/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 { 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 { 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()); diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 45b49dc46b897..44d39a870efcc 100644 --- a/bin/node/runtime/src/lib.rs +++ b/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::::new(), frame_system::CheckSpecVersion::::new(), frame_system::CheckTxVersion::::new(), frame_system::CheckGenesis::::new(), @@ -1334,6 +1335,7 @@ pub type BlockId = generic::BlockId; /// /// [`sign`]: <../../testing/src/keyring.rs.html> pub type SignedExtra = ( + frame_system::CheckNonZeroSender, frame_system::CheckSpecVersion, frame_system::CheckTxVersion, frame_system::CheckGenesis, diff --git a/bin/node/test-runner-example/src/lib.rs b/bin/node/test-runner-example/src/lib.rs index 68c14b73bf562..e247fca223204 100644 --- a/bin/node/test-runner-example/src/lib.rs +++ b/bin/node/test-runner-example/src/lib.rs @@ -69,6 +69,7 @@ impl ChainInfo for NodeTemplateChainInfo { from: ::AccountId, ) -> Self::SignedExtras { ( + frame_system::CheckNonZeroSender::::new(), frame_system::CheckSpecVersion::::new(), frame_system::CheckTxVersion::::new(), frame_system::CheckGenesis::::new(), diff --git a/bin/node/testing/src/keyring.rs b/bin/node/testing/src/keyring.rs index 1040e90c4d5d4..3e6dff301fc45 100644 --- a/bin/node/testing/src/keyring.rs +++ b/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(), diff --git a/frame/system/src/extensions/check_non_zero_sender.rs b/frame/system/src/extensions/check_non_zero_sender.rs new file mode 100644 index 0000000000000..1d45ae17cb7ac --- /dev/null +++ b/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(PhantomData); + +impl sp_std::fmt::Debug for CheckNonZeroSender { + #[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 CheckNonZeroSender { + /// Create new `SignedExtension` to check runtime version. + pub fn new() -> Self { + Self(sp_std::marker::PhantomData) + } +} + +impl SignedExtension for CheckNonZeroSender +where + T::Call: Dispatchable, +{ + 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, + _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::::new().validate(&0, CALL, &info, len), + InvalidTransaction::BadSigner + ); + assert_ok!(CheckNonZeroSender::::new().validate(&1, CALL, &info, len)); + }) + } +} diff --git a/frame/system/src/extensions/mod.rs b/frame/system/src/extensions/mod.rs index 0af9722e475d1..7eaff34c1d7f3 100644 --- a/frame/system/src/extensions/mod.rs +++ b/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; diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index aac104fa8f765..7603fe6541415 100644 --- a/frame/system/src/lib.rs +++ b/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, }; diff --git a/primitives/runtime/src/transaction_validity.rs b/primitives/runtime/src/transaction_validity.rs index e114bb5985460..cf5d925975996 100644 --- a/primitives/runtime/src/transaction_validity.rs +++ b/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 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", } } } diff --git a/test-utils/test-runner/src/lib.rs b/test-utils/test-runner/src/lib.rs index ca2c518fd6926..9f51442ed743b 100644 --- a/test-utils/test-runner/src/lib.rs +++ b/test-utils/test-runner/src/lib.rs @@ -106,6 +106,7 @@ //! let nonce = frame_system::Pallet::::account_nonce(from); //! //! ( +//! frame_system::CheckNonZeroSender::::new(), //! frame_system::CheckSpecVersion::::new(), //! frame_system::CheckTxVersion::::new(), //! frame_system::CheckGenesis::::new(),