Skip to content

Commit

Permalink
Revert "Make CheckNonce refuse transactions signed by accounts with n…
Browse files Browse the repository at this point in the history
…o providers (paritytech#1578)"

This reverts commit 93d9c8c.

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.
  • Loading branch information
kayabaNerve committed Nov 6, 2023
1 parent 1760168 commit c2750ba
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 61 deletions.
2 changes: 1 addition & 1 deletion substrate/bin/node/executor/tests/submit_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() };
<frame_system::Account<Runtime>>::insert(&address, account);

// check validity
Expand Down
23 changes: 16 additions & 7 deletions substrate/client/service/test/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand All @@ -61,17 +62,22 @@ fn construct_block(
backend: &InMemoryBackend<BlakeTwo256>,
number: BlockNumber,
parent_hash: Hash,
state_root: Hash,
txs: Vec<Transfer>,
) -> Vec<u8> {
) -> (Vec<u8>, Hash) {
let transactions = txs.into_iter().map(|tx| tx.into_unchecked_extrinsic()).collect::<Vec<_>>();

let iter = transactions.iter().map(Encode::encode);
let extrinsics_root = LayoutV0::<BlakeTwo256>::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");
Expand Down Expand Up @@ -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<BlakeTwo256>) -> Vec<u8> {
fn block1(genesis_hash: Hash, backend: &InMemoryBackend<BlakeTwo256>) -> (Vec<u8>, 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,
Expand Down Expand Up @@ -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");

Expand Down
56 changes: 3 additions & 53 deletions substrate/frame/system/src/extensions/check_nonce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -81,10 +81,6 @@ where
_len: usize,
) -> Result<(), TransactionValidityError> {
let mut account = crate::Account::<T>::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
Expand All @@ -105,11 +101,8 @@ where
_info: &DispatchInfoOf<Self::Call>,
_len: usize,
) -> TransactionValidity {
// check index
let account = crate::Account::<T>::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()
}
Expand Down Expand Up @@ -145,7 +138,7 @@ mod tests {
crate::AccountInfo {
nonce: 1,
consumers: 0,
providers: 1,
providers: 0,
sufficients: 0,
data: 0,
},
Expand All @@ -172,47 +165,4 @@ mod tests {
);
})
}

#[test]
fn signed_ext_check_nonce_requires_provider() {
new_test_ext().execute_with(|| {
crate::Account::<Test>::insert(
2,
crate::AccountInfo {
nonce: 1,
consumers: 0,
providers: 1,
sufficients: 0,
data: 0,
},
);
crate::Account::<Test>::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::<Test>(1).validate(&1, CALL, &info, len),
InvalidTransaction::Payment
);
assert_noop!(
CheckNonce::<Test>(1).pre_dispatch(&1, CALL, &info, len),
InvalidTransaction::Payment
);
// Non-zero providers
assert_ok!(CheckNonce::<Test>(1).validate(&2, CALL, &info, len));
assert_ok!(CheckNonce::<Test>(1).pre_dispatch(&2, CALL, &info, len));
// Non-zero sufficients
assert_ok!(CheckNonce::<Test>(1).validate(&3, CALL, &info, len));
assert_ok!(CheckNonce::<Test>(1).pre_dispatch(&3, CALL, &info, len));
})
}
}

0 comments on commit c2750ba

Please sign in to comment.