Skip to content

CheckOnlySudoAccount: Provide some tags #7838

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bkchr merged 7 commits into from
Mar 10, 2025
Merged

CheckOnlySudoAccount: Provide some tags #7838

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
dccf002
`CheckOnlySudoAccount`: Provide some tags
bkchr Mar 6, 2025
1904e0b
Update from github-actions[bot] running command 'prdoc --audience run…
github-actions[bot] Mar 6, 2025
4d63c36
Update pr_7838.prdoc
bkchr Mar 6, 2025
8688eb2
vec
bkchr Mar 6, 2025
87043d7
Merge remote-tracking branch 'refs/remotes/origin/bkchr-sudo-extension'
bkchr Mar 6, 2025
58de5fc
Review comments
bkchr Mar 7, 2025
91fecc8
Remove `CheckNonce`
bkchr Mar 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion cumulus/parachains/runtimes/glutton/glutton-westend/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,6 @@ pub type TxExtension = (
frame_system::CheckTxVersion<Runtime>,
frame_system::CheckGenesis<Runtime>,
frame_system::CheckEra<Runtime>,
frame_system::CheckNonce<Runtime>,
frame_system::CheckWeight<Runtime>,
frame_system::WeightReclaim<Runtime>,
);
Expand Down
7 changes: 7 additions & 0 deletions prdoc/pr_7838.prdoc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
title: '`CheckOnlySudoAccount`: Provide some tags'
doc:
- audience: Runtime User
description: Let `CheckOnlySudoAccount` provide some tags to make the tx pool happy.
crates:
- name: pallet-sudo
bump: patch
8 changes: 5 additions & 3 deletions substrate/frame/sudo/src/extension.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,14 @@
// limitations under the License.

use crate::{Config, Key};
use alloc::vec;
use codec::{Decode, DecodeWithMemTracking, Encode};
use core::{fmt, marker::PhantomData};
use frame_support::{dispatch::DispatchInfo, ensure, pallet_prelude::TransactionSource};
use scale_info::TypeInfo;
use sp_runtime::{
impl_tx_ext_default,
traits::{AsSystemOriginSigner, DispatchInfoOf, Dispatchable, TransactionExtension},
traits::{AsSystemOriginSigner, DispatchInfoOf, Dispatchable, Hash, TransactionExtension},
transaction_validity::{
InvalidTransaction, TransactionPriority, TransactionValidityError, UnknownTransaction,
ValidTransaction,
Expand All @@ -31,7 +32,7 @@ use sp_runtime::{

/// Ensure that signed transactions are only valid if they are signed by sudo account.
///
/// In the initial phase of a chain without any tokens you can not prevent accounts from sending
/// In the initial phase of a chain without any tokens you cannot prevent accounts from sending
/// transactions.
/// These transactions would enter the transaction pool as the succeed the validation, but would
/// fail on applying them as they are not allowed/disabled/whatever. This would be some huge dos
Expand Down Expand Up @@ -89,7 +90,7 @@ where
fn validate(
&self,
origin: <<T as frame_system::Config>::RuntimeCall as Dispatchable>::RuntimeOrigin,
_call: &<T as frame_system::Config>::RuntimeCall,
call: &<T as frame_system::Config>::RuntimeCall,
info: &DispatchInfoOf<<T as frame_system::Config>::RuntimeCall>,
_len: usize,
_self_implicit: Self::Implicit,
Expand All @@ -110,6 +111,7 @@ where
Ok((

@gui1117 gui1117 Mar 10, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also add a sufficient reference to the signer here automatically. Sounds cleaner than removing CheckNonce.

@bkchr bkchr Mar 10, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What you mean by this?

@gui1117 gui1117 Mar 10, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We remove CheckNonce because CheckNonce requires the account to exist.

But if we put CheckOnlySudoAccount before CheckNonce and CheckOnlySudoAccount creates the account with inc_sufficient (if it doesn't exist yet):

polkadot-sdk/substrate/frame/system/src/lib.rs

Line 1656 in 007565a

pub fn inc_sufficients(who: &T::AccountId) -> IncRefStatus {

Then the account exists and the nonce is checked.

@bkchr bkchr Mar 10, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And when is dec_sufficients called? :D

@gui1117 gui1117 Mar 10, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes true forget it :-), he could live forever

ValidTransaction {
priority: info.total_weight().ref_time() as TransactionPriority,
provides: vec![(who, T::Hashing::hash_of(call)).encode()],
..Default::default()
},
(),
Expand Down