From 70938c80d2388376d4a88370a6c5ca8b8d7719be Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Thu, 2 Dec 2021 11:09:26 -0500 Subject: [PATCH 01/11] Introduce CheckNonZeroSender --- bin/node/runtime/src/lib.rs | 2 ++ frame/system/src/extensions/mod.rs | 1 + frame/system/src/lib.rs | 2 +- primitives/runtime/src/transaction_validity.rs | 3 +++ 4 files changed, 7 insertions(+), 1 deletion(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 2cf6548b79211..ccbacb009a911 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -973,6 +973,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(), @@ -1322,6 +1323,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/frame/system/src/extensions/mod.rs b/frame/system/src/extensions/mod.rs index 0af9722e475d1..53cbc2a4035c6 100644 --- a/frame/system/src/extensions/mod.rs +++ b/frame/system/src/extensions/mod.rs @@ -18,6 +18,7 @@ pub mod check_genesis; pub mod check_mortality; pub mod check_nonce; +pub mod check_non_zero_sender; pub mod check_spec_version; pub mod check_tx_version; pub mod check_weight; diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index aac104fa8f765..556cf2e8ee6fe 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -117,7 +117,7 @@ pub mod weights; pub use extensions::{ check_genesis::CheckGenesis, check_mortality::CheckMortality, check_nonce::CheckNonce, check_spec_version::CheckSpecVersion, check_tx_version::CheckTxVersion, - check_weight::CheckWeight, + check_weight::CheckWeight, check_non_zero_sender::CheckNonZeroSender, }; // Backward compatible re-export. pub use extensions::check_mortality::CheckMortality as CheckEra; 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", } } } From 28d97d55ae1a11aa12f3a7dd8e9a16f14c9b7520 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Thu, 2 Dec 2021 11:17:22 -0500 Subject: [PATCH 02/11] Missing file --- .../src/extensions/check_non_zero_sender.rs | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 frame/system/src/extensions/check_non_zero_sender.rs 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..99da6f0e2f2c3 --- /dev/null +++ b/frame/system/src/extensions/check_non_zero_sender.rs @@ -0,0 +1,103 @@ +// 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, One, SignedExtension}, + transaction_validity::{ + InvalidTransaction, TransactionLongevity, TransactionValidity, TransactionValidityError, + ValidTransaction, + }, +}; +use sp_std::{prelude::*, marker::PhantomData}; + +/// 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 = "CheckNonZero"; + + 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), + ); + }) + } +} From 09eac590e610bba98169b5a30d7ee946d34c5f9f Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Thu, 2 Dec 2021 15:23:49 -0500 Subject: [PATCH 03/11] Formatting --- frame/system/src/extensions/check_non_zero_sender.rs | 8 +++----- frame/system/src/extensions/mod.rs | 2 +- frame/system/src/lib.rs | 5 +++-- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/frame/system/src/extensions/check_non_zero_sender.rs b/frame/system/src/extensions/check_non_zero_sender.rs index 99da6f0e2f2c3..eb18fdeb394be 100644 --- a/frame/system/src/extensions/check_non_zero_sender.rs +++ b/frame/system/src/extensions/check_non_zero_sender.rs @@ -26,7 +26,7 @@ use sp_runtime::{ ValidTransaction, }, }; -use sp_std::{prelude::*, marker::PhantomData}; +use sp_std::{marker::PhantomData, prelude::*}; /// Check to ensure that the sender is not the zero address. #[derive(Encode, Decode, Clone, Eq, PartialEq, TypeInfo)] @@ -74,7 +74,7 @@ where _len: usize, ) -> TransactionValidity { if who.using_encoded(|d| d.into_iter().all(|x| *x == 0)) { - return Err(TransactionValidityError::Invalid(InvalidTransaction::BadSigner)); + return Err(TransactionValidityError::Invalid(InvalidTransaction::BadSigner)) } Ok(ValidTransaction::default()) } @@ -95,9 +95,7 @@ mod tests { CheckNonZeroSender::::new().validate(&0, CALL, &info, len), InvalidTransaction::BadSigner ); - assert_ok!( - CheckNonZeroSender::::new().validate(&1, CALL, &info, len), - ); + 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 53cbc2a4035c6..7eaff34c1d7f3 100644 --- a/frame/system/src/extensions/mod.rs +++ b/frame/system/src/extensions/mod.rs @@ -17,8 +17,8 @@ pub mod check_genesis; pub mod check_mortality; -pub mod check_nonce; pub mod check_non_zero_sender; +pub mod check_nonce; pub mod check_spec_version; pub mod check_tx_version; pub mod check_weight; diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index 556cf2e8ee6fe..7603fe6541415 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -115,9 +115,10 @@ 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, check_non_zero_sender::CheckNonZeroSender, + check_weight::CheckWeight, }; // Backward compatible re-export. pub use extensions::check_mortality::CheckMortality as CheckEra; From 94471026bd73429f41e06d7048fbaabf9c779b7f Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Thu, 2 Dec 2021 15:25:47 -0500 Subject: [PATCH 04/11] Fixes --- .../system/src/extensions/check_non_zero_sender.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/frame/system/src/extensions/check_non_zero_sender.rs b/frame/system/src/extensions/check_non_zero_sender.rs index eb18fdeb394be..2c6d37292db72 100644 --- a/frame/system/src/extensions/check_non_zero_sender.rs +++ b/frame/system/src/extensions/check_non_zero_sender.rs @@ -16,17 +16,14 @@ // limitations under the License. use crate::Config; +use sp_std::{marker::PhantomData, prelude::*}; use codec::{Decode, Encode}; -use frame_support::weights::DispatchInfo; use scale_info::TypeInfo; -use sp_runtime::{ - traits::{DispatchInfoOf, Dispatchable, One, SignedExtension}, - transaction_validity::{ - InvalidTransaction, TransactionLongevity, TransactionValidity, TransactionValidityError, - ValidTransaction, - }, +use sp_runtime::traits::{DispatchInfoOf, Dispatchable, SignedExtension}; +use sp_runtime::transaction_validity::{ + InvalidTransaction, TransactionValidity, TransactionValidityError, ValidTransaction, }; -use sp_std::{marker::PhantomData, prelude::*}; +use frame_support::weights::DispatchInfo; /// Check to ensure that the sender is not the zero address. #[derive(Encode, Decode, Clone, Eq, PartialEq, TypeInfo)] From 51a725b94875d4b3dc655943ed2c121ed968e3c5 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Thu, 2 Dec 2021 15:29:22 -0500 Subject: [PATCH 05/11] Formatting --- frame/system/src/extensions/check_non_zero_sender.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/frame/system/src/extensions/check_non_zero_sender.rs b/frame/system/src/extensions/check_non_zero_sender.rs index 2c6d37292db72..aff7957d52bb3 100644 --- a/frame/system/src/extensions/check_non_zero_sender.rs +++ b/frame/system/src/extensions/check_non_zero_sender.rs @@ -16,14 +16,16 @@ // limitations under the License. use crate::Config; -use sp_std::{marker::PhantomData, prelude::*}; use codec::{Decode, Encode}; +use frame_support::weights::DispatchInfo; use scale_info::TypeInfo; -use sp_runtime::traits::{DispatchInfoOf, Dispatchable, SignedExtension}; -use sp_runtime::transaction_validity::{ - InvalidTransaction, TransactionValidity, TransactionValidityError, ValidTransaction, +use sp_runtime::{ + traits::{DispatchInfoOf, Dispatchable, SignedExtension}, + transaction_validity::{ + InvalidTransaction, TransactionValidity, TransactionValidityError, ValidTransaction, + }, }; -use frame_support::weights::DispatchInfo; +use sp_std::{marker::PhantomData, prelude::*}; /// Check to ensure that the sender is not the zero address. #[derive(Encode, Decode, Clone, Eq, PartialEq, TypeInfo)] From e5f1f2aaae1841029574170caf7f56c8bb35051d Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Thu, 2 Dec 2021 17:06:32 -0400 Subject: [PATCH 06/11] some fixes to compile --- bin/node/cli/src/service.rs | 4 ++++ bin/node/test-runner-example/src/lib.rs | 1 + bin/node/testing/src/keyring.rs | 1 + test-utils/test-runner/src/lib.rs | 1 + 4 files changed, 7 insertions(+) diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index fec91a9b67cc4..4245c8b360546 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::CheckSpecVersion::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, 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/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(), From 837abd719564cf2ed24030154322ba5deb73f387 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Thu, 2 Dec 2021 17:07:14 -0400 Subject: [PATCH 07/11] Update frame/system/src/extensions/check_non_zero_sender.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- frame/system/src/extensions/check_non_zero_sender.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/system/src/extensions/check_non_zero_sender.rs b/frame/system/src/extensions/check_non_zero_sender.rs index aff7957d52bb3..1e295bbb9ed9b 100644 --- a/frame/system/src/extensions/check_non_zero_sender.rs +++ b/frame/system/src/extensions/check_non_zero_sender.rs @@ -94,7 +94,7 @@ mod tests { CheckNonZeroSender::::new().validate(&0, CALL, &info, len), InvalidTransaction::BadSigner ); - assert_ok!(CheckNonZeroSender::::new().validate(&1, CALL, &info, len),); + assert_ok!(CheckNonZeroSender::::new().validate(&1, CALL, &info, len)); }) } } From d8767dd7cdc90e2e50ae5877b7149b763250e3d0 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Thu, 2 Dec 2021 16:10:30 -0500 Subject: [PATCH 08/11] Fixes --- bin/node/cli/src/service.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index fec91a9b67cc4..4e913f0219aad 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(), @@ -719,6 +720,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 +729,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, From 383f2828227b075fd6b76fae769e048f06fb7dd4 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Thu, 2 Dec 2021 16:11:50 -0500 Subject: [PATCH 09/11] Fixes --- frame/system/src/extensions/check_non_zero_sender.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/system/src/extensions/check_non_zero_sender.rs b/frame/system/src/extensions/check_non_zero_sender.rs index 1e295bbb9ed9b..1d45ae17cb7ac 100644 --- a/frame/system/src/extensions/check_non_zero_sender.rs +++ b/frame/system/src/extensions/check_non_zero_sender.rs @@ -59,7 +59,7 @@ where type Call = T::Call; type AdditionalSigned = (); type Pre = (); - const IDENTIFIER: &'static str = "CheckNonZero"; + const IDENTIFIER: &'static str = "CheckNonZeroSender"; fn additional_signed(&self) -> sp_std::result::Result<(), TransactionValidityError> { Ok(()) From 3cbfdf6669c5418e7a10f942f19c06f767e823ab Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Thu, 2 Dec 2021 17:40:23 -0400 Subject: [PATCH 10/11] another fix --- bin/node/cli/src/service.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index 3af9eb402dd96..1dfce9331b752 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -742,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(); From a257ff3b822837e81c5a5ac05cdfa7ea384e1d3e Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Thu, 2 Dec 2021 17:39:20 -0500 Subject: [PATCH 11/11] Formatting --- bin/node/executor/tests/submit_transaction.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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());