Skip to content

chore: remove sudo pallet#1570

Merged
ipapandinas merged 7 commits intomasterfrom
chore/remove-sudo
Dec 18, 2025
Merged

chore: remove sudo pallet#1570
ipapandinas merged 7 commits intomasterfrom
chore/remove-sudo

Conversation

@ipapandinas
Copy link
Contributor

Pull Request Summary

Closes #1535

@ipapandinas ipapandinas added astar Related to Astar runtime This PR/Issue is related to the topic “runtime”. labels Dec 5, 2025
Comment on lines -1004 to -1008
impl pallet_sudo::Config for Runtime {
type RuntimeEvent = RuntimeEvent;
type RuntimeCall = RuntimeCall;
type WeightInfo = pallet_sudo::weights::SubstrateWeight<Runtime>;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you put this under some feature flag?

I'm fine with not having this on-chain, in the live runtime, but quickly swapping out live runtime with the one compiled locally with the enabled astar-sudo flag can be useful for quick debugging.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added an astar-sudo feature flag to conditionally enable the Sudo pallet. However, construct_runtime! doesn't support #[cfg] attributes on individual pallets, leading to compilation issues. I also tried migrating to the newer #[frame_support::runtime] macro, but again same issues.

So I'm using a simple construct_astar_runtime! wrapper macro as a pragmatic solution to keep the code DRY. Happy to rework if you have a better approach or have dealt with feature-gated pallet inclusion before.

Copy link
Contributor

Choose a reason for hiding this comment

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

So I'm using a simple construct_astar_runtime! wrapper macro as a pragmatic solution to keep the code DRY. Happy to rework if you have a better approach or have dealt with feature-gated pallet inclusion before.

What you might want to try is to use the new syntax for constructing the runtime, i.e. the new macro.
It's used in polkadot-sdk runtime template for example.

I did a quick try with it and managed to disable sudo pallet in another project in that way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What you might want to try is to use the new syntax for constructing the runtime, i.e. the new macro.
It's used in polkadot-sdk runtime template for example.

Are you suggesting this?

#[frame_support::runtime]
mod runtime {
    #[runtime::runtime]
    #[runtime::derive(
        RuntimeCall,
        RuntimeEvent,
        RuntimeError,
        RuntimeOrigin,
        RuntimeFreezeReason,
        RuntimeHoldReason,
        RuntimeSlashReason,
        RuntimeLockId,
        RuntimeTask,
    )]
    pub struct Runtime;

    #[runtime::pallet_index(10)]
    pub type System = frame_system;

    ...
    
    #[cfg(feature = "astar-sudo")]
    #[runtime::pallet_index(99)]
    pub type Sudo = pallet_sudo;
}

It doesn't compile, I think the entire runtime must be feature-gated:

https://github.com/paritytech/polkadot-sdk/blob/ec03705c977b42995de78fa897ce46fae8f5b402/substrate/frame/session/src/mock.rs#L75-L94

Copy link
Contributor

Choose a reason for hiding this comment

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

For me it compiled.
It might be due to the newer version of polkadot-sdk though.

The link you provided uses the old macro, and I agree and know that it doesn't support gating on the pallet level.

Dinonard
Dinonard previously approved these changes Dec 16, 2025
Copy link
Contributor

@Dinonard Dinonard left a comment

Choose a reason for hiding this comment

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

LGTM

We can revisit the macro after uplifts.

PierreOssun
PierreOssun previously approved these changes Dec 16, 2025
@ipapandinas ipapandinas dismissed stale reviews from PierreOssun and Dinonard via a7abb24 December 16, 2025 15:55

/// Unreleased migrations. Add new ones here:
pub type Unreleased = ();
pub type Unreleased = (pallet_collator_selection::migrations::LastAuthoredBlockCleanup<Runtime>,);
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't you also need the XC asset migration for Shibuya?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, I forgot we did some testing for AH in Shibuya too, I have added it now.

@github-actions
Copy link

Code Coverage

Package Line Rate Branch Rate Health
pallets/dapp-staking/rpc/runtime-api/src 0% 0%
pallets/xc-asset-config/src 56% 0%
pallets/collective-proxy/src 94% 0%
pallets/ethereum-checked/src 76% 0%
precompiles/dapp-staking/src/test 0% 0%
primitives/src 55% 0%
chain-extensions/types/unified-accounts/src 0% 0%
pallets/dynamic-evm-base-fee/src 85% 0%
pallets/vesting-mbm/src 87% 0%
pallets/collator-selection/src 82% 0%
pallets/democracy-mbm/src 30% 0%
pallets/astar-xcm-benchmarks/src/fungible 100% 0%
precompiles/substrate-ecdsa/src 67% 0%
chain-extensions/unified-accounts/src 0% 0%
pallets/dapp-staking/src 80% 0%
pallets/astar-xcm-benchmarks/src 86% 0%
precompiles/assets-erc20/src 77% 0%
precompiles/dispatch-lockdrop/src 83% 0%
pallets/astar-xcm-benchmarks/src/generic 100% 0%
pallets/static-price-provider/src 91% 0%
precompiles/dapp-staking/src 89% 0%
primitives/src/xcm 66% 0%
pallets/inflation/src 58% 0%
precompiles/unified-accounts/src 100% 0%
precompiles/xcm/src 69% 0%
pallets/price-aggregator/src 75% 0%
chain-extensions/types/assets/src 0% 0%
precompiles/sr25519/src 56% 0%
pallets/dapp-staking/src/benchmarking 95% 0%
chain-extensions/pallet-assets/src 54% 0%
pallets/dapp-staking/src/test 0% 0%
pallets/unified-accounts/src 80% 0%
Summary 72% (3888 / 5383) 0% (0 / 0)

Minimum allowed line rate is 50%

@ipapandinas ipapandinas merged commit 77a0608 into master Dec 18, 2025
8 checks passed
@ipapandinas ipapandinas deleted the chore/remove-sudo branch December 18, 2025 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

astar Related to Astar runtime This PR/Issue is related to the topic “runtime”.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove sudo pallet from Astar

3 participants