From dc6ff91d405e7af05f8d8a424571adea4612f602 Mon Sep 17 00:00:00 2001 From: open-junius Date: Thu, 9 May 2024 22:41:31 +0800 Subject: [PATCH 1/4] fix pays no check --- runtime/src/check_nonce.rs | 231 +++++++++++++++++++++++++++++++++++++ runtime/src/lib.rs | 3 +- 2 files changed, 233 insertions(+), 1 deletion(-) create mode 100644 runtime/src/check_nonce.rs diff --git a/runtime/src/check_nonce.rs b/runtime/src/check_nonce.rs new file mode 100644 index 000000000..dec471f79 --- /dev/null +++ b/runtime/src/check_nonce.rs @@ -0,0 +1,231 @@ +// This file is part of Substrate. + +// Copyright (C) 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 codec::{Decode, Encode}; +use frame_support::dispatch::{DispatchInfo, Pays}; +use frame_system::Config; +use scale_info::TypeInfo; +use sp_runtime::{ + traits::{DispatchInfoOf, Dispatchable, One, SignedExtension, Zero}, + transaction_validity::{ + InvalidTransaction, TransactionLongevity, TransactionValidity, TransactionValidityError, + ValidTransaction, + }, +}; +use sp_std::vec; + +/// Nonce check and increment to give replay protection for transactions. +/// +/// # Transaction Validity +/// +/// This extension affects `requires` and `provides` tags of validity, but DOES NOT +/// set the `priority` field. Make sure that AT LEAST one of the signed extension sets +/// some kind of priority upon validating transactions. +#[derive(Encode, Decode, Clone, Eq, PartialEq, TypeInfo)] +#[scale_info(skip_type_params(T))] +pub struct CheckNonce(#[codec(compact)] pub T::Nonce); + +impl CheckNonce { + /// utility constructor. Used only in client/factory code. + pub fn from(nonce: T::Nonce) -> Self { + Self(nonce) + } +} + +impl sp_std::fmt::Debug for CheckNonce { + #[cfg(feature = "std")] + fn fmt(&self, f: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result { + write!(f, "CheckNonce({})", self.0) + } + + #[cfg(not(feature = "std"))] + fn fmt(&self, _: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result { + Ok(()) + } +} + +impl SignedExtension for CheckNonce +where + T::RuntimeCall: Dispatchable, +{ + type AccountId = T::AccountId; + type Call = T::RuntimeCall; + type AdditionalSigned = (); + type Pre = (); + const IDENTIFIER: &'static str = "CheckNonce"; + + fn additional_signed(&self) -> sp_std::result::Result<(), TransactionValidityError> { + Ok(()) + } + + fn pre_dispatch( + self, + who: &Self::AccountId, + _call: &Self::Call, + info: &DispatchInfoOf, + _len: usize, + ) -> Result<(), TransactionValidityError> { + // let dispatch_info = ::get_dispatch_info(call); + let mut account = frame_system::Account::::get(who); + match info.pays_fee { + Pays::Yes => { + if account.providers.is_zero() && account.sufficients.is_zero() { + // Nonce storage not paid for + return Err(InvalidTransaction::Payment.into()); + } + } + // not check providers and sufficients for Pays::No extrinsic + Pays::No => {} + } + + if self.0 != account.nonce { + return Err(if self.0 < account.nonce { + InvalidTransaction::Stale + } else { + InvalidTransaction::Future + } + .into()); + } + account.nonce += T::Nonce::one(); + frame_system::Account::::insert(who, account); + Ok(()) + } + + fn validate( + &self, + who: &Self::AccountId, + _call: &Self::Call, + info: &DispatchInfoOf, + _len: usize, + ) -> TransactionValidity { + let account = frame_system::Account::::get(who); + match info.pays_fee { + Pays::Yes => { + if account.providers.is_zero() && account.sufficients.is_zero() { + // Nonce storage not paid for + return Err(InvalidTransaction::Payment.into()); + } + } + // not check providers and sufficients for Pays::No extrinsic + Pays::No => {} + } + if self.0 < account.nonce { + return InvalidTransaction::Stale.into(); + } + + let provides = vec![Encode::encode(&(who, self.0))]; + let requires = if account.nonce < self.0 { + vec![Encode::encode(&(who, self.0 - One::one()))] + } else { + vec![] + }; + + Ok(ValidTransaction { + priority: 0, + requires, + provides, + longevity: TransactionLongevity::max_value(), + propagate: true, + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::mock::{new_test_ext, Test, CALL}; + use frame_support::{assert_noop, assert_ok}; + + #[test] + fn signed_ext_check_nonce_works() { + new_test_ext().execute_with(|| { + crate::Account::::insert( + 1, + crate::AccountInfo { + nonce: 1, + consumers: 0, + providers: 1, + sufficients: 0, + data: 0, + }, + ); + let info = DispatchInfo::default(); + let len = 0_usize; + // stale + assert_noop!( + CheckNonce::(0).validate(&1, CALL, &info, len), + InvalidTransaction::Stale + ); + assert_noop!( + CheckNonce::(0).pre_dispatch(&1, CALL, &info, len), + InvalidTransaction::Stale + ); + // correct + assert_ok!(CheckNonce::(1).validate(&1, CALL, &info, len)); + assert_ok!(CheckNonce::(1).pre_dispatch(&1, CALL, &info, len)); + // future + assert_ok!(CheckNonce::(5).validate(&1, CALL, &info, len)); + assert_noop!( + CheckNonce::(5).pre_dispatch(&1, CALL, &info, len), + InvalidTransaction::Future + ); + }) + } + + #[test] + fn signed_ext_check_nonce_requires_provider() { + new_test_ext().execute_with(|| { + crate::Account::::insert( + 2, + crate::AccountInfo { + nonce: 1, + consumers: 0, + providers: 1, + sufficients: 0, + data: 0, + }, + ); + crate::Account::::insert( + 3, + crate::AccountInfo { + nonce: 1, + consumers: 0, + providers: 0, + sufficients: 1, + data: 0, + }, + ); + let info = DispatchInfo::default(); + let len = 0_usize; + // Both providers and sufficients zero + assert_noop!( + CheckNonce::(1).validate(&1, CALL, &info, len), + InvalidTransaction::Payment + ); + assert_noop!( + CheckNonce::(1).pre_dispatch(&1, CALL, &info, len), + InvalidTransaction::Payment + ); + // Non-zero providers + assert_ok!(CheckNonce::(1).validate(&2, CALL, &info, len)); + assert_ok!(CheckNonce::(1).pre_dispatch(&2, CALL, &info, len)); + // Non-zero sufficients + assert_ok!(CheckNonce::(1).validate(&3, CALL, &info, len)); + assert_ok!(CheckNonce::(1).pre_dispatch(&3, CALL, &info, len)); + }) + } +} diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index ffbbfb87d..d6ef34195 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -6,6 +6,7 @@ #[cfg(feature = "std")] include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs")); +mod check_nonce; mod migrations; use codec::{Decode, Encode, MaxEncodedLen}; @@ -1177,7 +1178,7 @@ pub type SignedExtra = ( frame_system::CheckTxVersion, frame_system::CheckGenesis, frame_system::CheckEra, - frame_system::CheckNonce, + check_nonce::CheckNonce, frame_system::CheckWeight, pallet_transaction_payment::ChargeTransactionPayment, pallet_subtensor::SubtensorSignedExtension, From deb32ddd22dfc57781d442b57a76976ac7b3517f Mon Sep 17 00:00:00 2001 From: open-junius Date: Thu, 9 May 2024 22:50:56 +0800 Subject: [PATCH 2/4] remove test --- runtime/src/check_nonce.rs | 86 -------------------------------------- 1 file changed, 86 deletions(-) diff --git a/runtime/src/check_nonce.rs b/runtime/src/check_nonce.rs index dec471f79..225c717b2 100644 --- a/runtime/src/check_nonce.rs +++ b/runtime/src/check_nonce.rs @@ -143,89 +143,3 @@ where }) } } - -#[cfg(test)] -mod tests { - use super::*; - use crate::mock::{new_test_ext, Test, CALL}; - use frame_support::{assert_noop, assert_ok}; - - #[test] - fn signed_ext_check_nonce_works() { - new_test_ext().execute_with(|| { - crate::Account::::insert( - 1, - crate::AccountInfo { - nonce: 1, - consumers: 0, - providers: 1, - sufficients: 0, - data: 0, - }, - ); - let info = DispatchInfo::default(); - let len = 0_usize; - // stale - assert_noop!( - CheckNonce::(0).validate(&1, CALL, &info, len), - InvalidTransaction::Stale - ); - assert_noop!( - CheckNonce::(0).pre_dispatch(&1, CALL, &info, len), - InvalidTransaction::Stale - ); - // correct - assert_ok!(CheckNonce::(1).validate(&1, CALL, &info, len)); - assert_ok!(CheckNonce::(1).pre_dispatch(&1, CALL, &info, len)); - // future - assert_ok!(CheckNonce::(5).validate(&1, CALL, &info, len)); - assert_noop!( - CheckNonce::(5).pre_dispatch(&1, CALL, &info, len), - InvalidTransaction::Future - ); - }) - } - - #[test] - fn signed_ext_check_nonce_requires_provider() { - new_test_ext().execute_with(|| { - crate::Account::::insert( - 2, - crate::AccountInfo { - nonce: 1, - consumers: 0, - providers: 1, - sufficients: 0, - data: 0, - }, - ); - crate::Account::::insert( - 3, - crate::AccountInfo { - nonce: 1, - consumers: 0, - providers: 0, - sufficients: 1, - data: 0, - }, - ); - let info = DispatchInfo::default(); - let len = 0_usize; - // Both providers and sufficients zero - assert_noop!( - CheckNonce::(1).validate(&1, CALL, &info, len), - InvalidTransaction::Payment - ); - assert_noop!( - CheckNonce::(1).pre_dispatch(&1, CALL, &info, len), - InvalidTransaction::Payment - ); - // Non-zero providers - assert_ok!(CheckNonce::(1).validate(&2, CALL, &info, len)); - assert_ok!(CheckNonce::(1).pre_dispatch(&2, CALL, &info, len)); - // Non-zero sufficients - assert_ok!(CheckNonce::(1).validate(&3, CALL, &info, len)); - assert_ok!(CheckNonce::(1).pre_dispatch(&3, CALL, &info, len)); - }) - } -} From d241191dc8e6b3924e6df32227726b3044992ec3 Mon Sep 17 00:00:00 2001 From: open-junius Date: Thu, 9 May 2024 23:01:01 +0800 Subject: [PATCH 3/4] fix benchmark --- node/src/benchmarking.rs | 3 ++- runtime/src/lib.rs | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/node/src/benchmarking.rs b/node/src/benchmarking.rs index 218670609..ba176e15f 100644 --- a/node/src/benchmarking.rs +++ b/node/src/benchmarking.rs @@ -5,6 +5,7 @@ use crate::service::FullClient; use node_subtensor_runtime as runtime; +use node_subtensor_runtime::check_nonce; use node_subtensor_runtime::pallet_subtensor; use runtime::{AccountId, Balance, BalancesCall, SystemCall}; use sc_cli::Result; @@ -130,7 +131,7 @@ pub fn create_benchmark_extrinsic( period, best_block.saturated_into(), )), - frame_system::CheckNonce::::from(nonce), + check_nonce::CheckNonce::::from(nonce), frame_system::CheckWeight::::new(), pallet_transaction_payment::ChargeTransactionPayment::::from(0), pallet_subtensor::SubtensorSignedExtension::::new(), diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index d6ef34195..6dbe3738b 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -6,7 +6,7 @@ #[cfg(feature = "std")] include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs")); -mod check_nonce; +pub mod check_nonce; mod migrations; use codec::{Decode, Encode, MaxEncodedLen}; From f7653212e1b0e9e6d1ec0ff722dc31bf92f034e5 Mon Sep 17 00:00:00 2001 From: open-junius Date: Thu, 9 May 2024 23:21:40 +0800 Subject: [PATCH 4/4] remove commented code --- runtime/src/check_nonce.rs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/runtime/src/check_nonce.rs b/runtime/src/check_nonce.rs index 225c717b2..e6e992ccf 100644 --- a/runtime/src/check_nonce.rs +++ b/runtime/src/check_nonce.rs @@ -1,20 +1,3 @@ -// This file is part of Substrate. - -// Copyright (C) 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 codec::{Decode, Encode}; use frame_support::dispatch::{DispatchInfo, Pays}; use frame_system::Config; @@ -79,7 +62,6 @@ where info: &DispatchInfoOf, _len: usize, ) -> Result<(), TransactionValidityError> { - // let dispatch_info = ::get_dispatch_info(call); let mut account = frame_system::Account::::get(who); match info.pays_fee { Pays::Yes => {