From c2750ba3a01dd999844a7dda79156d95d0ba11c6 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Mon, 6 Nov 2023 15:00:45 -0500 Subject: [PATCH] Revert "Make CheckNonce refuse transactions signed by accounts with no providers (#1578)" This reverts commit 93d9c8c24e5d470e036caaa23df08dcf0c527dd6. Serai's coin-pallets doesn't set a provider as we plan to rip out these systems. Accordingly, this error would prevent users from submitting transactions. --- .../node/executor/tests/submit_transaction.rs | 2 +- .../client/service/test/src/client/mod.rs | 23 +++++--- .../system/src/extensions/check_nonce.rs | 56 +------------------ 3 files changed, 20 insertions(+), 61 deletions(-) diff --git a/substrate/bin/node/executor/tests/submit_transaction.rs b/substrate/bin/node/executor/tests/submit_transaction.rs index 29dd4aebfc40..02804f9bd39e 100644 --- a/substrate/bin/node/executor/tests/submit_transaction.rs +++ b/substrate/bin/node/executor/tests/submit_transaction.rs @@ -237,7 +237,7 @@ fn submitted_transaction_should_be_valid() { let author = extrinsic.signature.clone().unwrap().0; let address = author; let data = pallet_balances::AccountData { free: 5_000_000_000_000, ..Default::default() }; - let account = frame_system::AccountInfo { providers: 1, data, ..Default::default() }; + let account = frame_system::AccountInfo { data, ..Default::default() }; >::insert(&address, account); // check validity diff --git a/substrate/client/service/test/src/client/mod.rs b/substrate/client/service/test/src/client/mod.rs index 524415fca6e4..a240e8760704 100644 --- a/substrate/client/service/test/src/client/mod.rs +++ b/substrate/client/service/test/src/client/mod.rs @@ -39,6 +39,7 @@ use sp_runtime::{ }; use sp_state_machine::{backend::Backend as _, InMemoryBackend, OverlayedChanges, StateMachine}; use sp_storage::{ChildInfo, StorageKey}; +use sp_trie::{LayoutV0, TrieConfiguration}; use std::{collections::HashSet, sync::Arc}; use substrate_test_runtime::TestAPI; use substrate_test_runtime_client::{ @@ -61,17 +62,22 @@ fn construct_block( backend: &InMemoryBackend, number: BlockNumber, parent_hash: Hash, + state_root: Hash, txs: Vec, -) -> Vec { +) -> (Vec, Hash) { let transactions = txs.into_iter().map(|tx| tx.into_unchecked_extrinsic()).collect::>(); + let iter = transactions.iter().map(Encode::encode); + let extrinsics_root = LayoutV0::::ordered_trie_root(iter).into(); + let mut header = Header { parent_hash, number, - state_root: Default::default(), - extrinsics_root: Default::default(), + state_root, + extrinsics_root, digest: Digest { logs: vec![] }, }; + let hash = header.hash(); let mut overlay = OverlayedChanges::default(); let backend_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(backend); let runtime_code = backend_runtime_code.runtime_code().expect("Code is part of the backend"); @@ -118,16 +124,19 @@ fn construct_block( .unwrap(); header = Header::decode(&mut &ret_data[..]).unwrap(); - vec![].and(&Block { header, extrinsics: transactions }) + (vec![].and(&Block { header, extrinsics: transactions }), hash) } -fn block1(genesis_hash: Hash, backend: &InMemoryBackend) -> Vec { +fn block1(genesis_hash: Hash, backend: &InMemoryBackend) -> (Vec, Hash) { construct_block( backend, 1, genesis_hash, + array_bytes::hex_n_into_unchecked( + "25e5b37074063ab75c889326246640729b40d0c86932edc527bc80db0e04fe5c", + ), vec![Transfer { - from: AccountKeyring::One.into(), + from: AccountKeyring::Alice.into(), to: AccountKeyring::Two.into(), amount: 69 * DOLLARS, nonce: 0, @@ -166,7 +175,7 @@ fn construct_genesis_should_work() { let genesis_hash = insert_genesis_block(&mut storage); let backend = InMemoryBackend::from((storage, StateVersion::default())); - let b1data = block1(genesis_hash, &backend); + let (b1data, _b1hash) = block1(genesis_hash, &backend); let backend_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&backend); let runtime_code = backend_runtime_code.runtime_code().expect("Code is part of the backend"); diff --git a/substrate/frame/system/src/extensions/check_nonce.rs b/substrate/frame/system/src/extensions/check_nonce.rs index d1447d0a4f5c..2107ee771d27 100644 --- a/substrate/frame/system/src/extensions/check_nonce.rs +++ b/substrate/frame/system/src/extensions/check_nonce.rs @@ -21,7 +21,7 @@ use codec::{Decode, Encode}; use frame_support::dispatch::DispatchInfo; use scale_info::TypeInfo; use sp_runtime::{ - traits::{DispatchInfoOf, Dispatchable, One, SignedExtension, Zero}, + traits::{DispatchInfoOf, Dispatchable, One, SignedExtension}, transaction_validity::{ InvalidTransaction, TransactionLongevity, TransactionValidity, TransactionValidityError, ValidTransaction, @@ -81,10 +81,6 @@ where _len: usize, ) -> Result<(), TransactionValidityError> { let mut account = crate::Account::::get(who); - if account.providers.is_zero() && account.sufficients.is_zero() { - // Nonce storage not paid for - return Err(InvalidTransaction::Payment.into()) - } if self.0 != account.nonce { return Err(if self.0 < account.nonce { InvalidTransaction::Stale @@ -105,11 +101,8 @@ where _info: &DispatchInfoOf, _len: usize, ) -> TransactionValidity { + // check index let account = crate::Account::::get(who); - if account.providers.is_zero() && account.sufficients.is_zero() { - // Nonce storage not paid for - return InvalidTransaction::Payment.into() - } if self.0 < account.nonce { return InvalidTransaction::Stale.into() } @@ -145,7 +138,7 @@ mod tests { crate::AccountInfo { nonce: 1, consumers: 0, - providers: 1, + providers: 0, sufficients: 0, data: 0, }, @@ -172,47 +165,4 @@ mod tests { ); }) } - - #[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)); - }) - } }