Merge pull request #2665 from subspace/remove_sudo_requirement

Remove usage of SudoKey and use PermissionedAction to do specific actions on pallet-domains

crates/pallet-domains/src/benchmarking.rs

@@ -527,7 +527,7 @@ mod benchmarks {
 
     #[benchmark]
     fn instantiate_domain() {
-        let creator = T::SudoId::get();
+        let creator = account("domain_creator", 1, SEED);
         T::Currency::set_balance(
             &creator,
             T::DomainInstantiationDeposit::get() + T::MinNominatorStake::get(),
@@ -888,7 +888,7 @@ mod benchmarks {
     }
 
     fn register_domain<T: Config>() -> DomainId {
-        let creator = T::SudoId::get();
+        let creator = account("domain_creator", 1, SEED);
         T::Currency::set_balance(
             &creator,
             T::DomainInstantiationDeposit::get() + T::MinNominatorStake::get(),

crates/pallet-domains/src/lib.rs

@@ -350,10 +350,6 @@ mod pallet {
         /// Randomness source.
         type Randomness: RandomnessT<Self::Hash, BlockNumberFor<Self>>;
 
-        /// The sudo account id
-        #[pallet::constant]
-        type SudoId: Get<Self::AccountId>;
-
         /// The pallet-domains's pallet id.
         #[pallet::constant]
         type PalletId: Get<frame_support::PalletId>;
@@ -634,6 +630,11 @@ mod pallet {
     pub(super) type LatestSubmittedER<T: Config> =
         StorageMap<_, Identity, (DomainId, OperatorId), DomainBlockNumberFor<T>, ValueQuery>;
 
+    /// Storage for PermissionedActions for domain instantiation and other permissioned calls.
+    #[pallet::storage]
+    pub(super) type PermissionedActionAllowedBy<T: Config> =
+        StorageValue<_, sp_domains::PermissionedActionAllowedBy<T::AccountId>, OptionQuery>;
+
     #[derive(TypeInfo, Encode, Decode, PalletError, Debug, PartialEq)]
     pub enum BundleError {
         /// Can not find the operator for given operator id.
@@ -772,6 +773,8 @@ mod pallet {
         BlockTree(BlockTreeError),
         /// Bundle storage fund specific errors
         BundleStorageFund(BundleStorageFundError),
+        /// Permissioned action is not allowed by the caller.
+        PermissionedActionNotAllowed,
     }
 
     /// Reason for slashing an operator
@@ -1246,9 +1249,13 @@ mod pallet {
             origin: OriginFor<T>,
             domain_config: DomainConfig<T::AccountId, BalanceOf<T>>,
         ) -> DispatchResult {
-            ensure_root(origin)?;
-
-            let who = T::SudoId::get();
+            let who = ensure_signed(origin)?;
+            ensure!(
+                PermissionedActionAllowedBy::<T>::get()
+                    .map(|allowed_by| allowed_by.is_allowed(&who))
+                    .unwrap_or_default(),
+                Error::<T>::PermissionedActionNotAllowed
+            );
 
             let created_at = frame_system::Pallet::<T>::current_block_number();
 
@@ -1376,24 +1383,44 @@ mod pallet {
 
             Ok(Some(actual_weight).into())
         }
+
+        /// Update permissioned action allowed by storage by Sudo.
+        #[pallet::call_index(14)]
+        #[pallet::weight(<T as frame_system::Config>::DbWeight::get().reads_writes(0, 1))]
+        pub fn set_permissioned_action_allowed_by(
+            origin: OriginFor<T>,
+            permissioned_action_allowed_by: sp_domains::PermissionedActionAllowedBy<T::AccountId>,
+        ) -> DispatchResult {
+            ensure_root(origin)?;
+            PermissionedActionAllowedBy::<T>::put(permissioned_action_allowed_by);
+            Ok(())
+        }
     }
 
     #[pallet::genesis_config]
     pub struct GenesisConfig<T: Config> {
         pub genesis_domain: Option<GenesisDomain<T::AccountId, BalanceOf<T>>>,
+        pub permissioned_action_allowed_by:
+            Option<sp_domains::PermissionedActionAllowedBy<T::AccountId>>,
     }
 
     impl<T: Config> Default for GenesisConfig<T> {
         fn default() -> Self {
             GenesisConfig {
                 genesis_domain: None,
+                permissioned_action_allowed_by: None,
             }
         }
     }
 
     #[pallet::genesis_build]
     impl<T: Config> BuildGenesisConfig for GenesisConfig<T> {
         fn build(&self) {
+            if let Some(permissioned_action_allowed_by) =
+                self.permissioned_action_allowed_by.as_ref().cloned()
+            {
+                PermissionedActionAllowedBy::<T>::put(permissioned_action_allowed_by)
+            }
             if let Some(genesis_domain) = self.genesis_domain.as_ref().cloned() {
                 // Register the genesis domain runtime
                 let runtime_id = register_runtime_at_genesis::<T>(

crates/pallet-domains/src/tests.rs

@@ -279,7 +279,6 @@ impl pallet_domains::Config for Test {
     type MaxPendingStakingOperation = MaxPendingStakingOperation;
     type MaxNominators = MaxNominators;
     type Randomness = MockRandomness;
-    type SudoId = ();
     type PalletId = DomainsPalletId;
     type StorageFee = DummyStorageFee;
     type BlockSlot = DummyBlockSlot;
@@ -577,6 +576,10 @@ pub(crate) fn run_to_block<T: Config>(block_number: BlockNumberFor<T>, parent_ha
 
 pub(crate) fn register_genesis_domain(creator: u128, operator_ids: Vec<OperatorId>) -> DomainId {
     let raw_genesis_storage = RawGenesis::dummy(vec![1, 2, 3, 4]).encode();
+    assert_ok!(crate::Pallet::<Test>::set_permissioned_action_allowed_by(
+        RawOrigin::Root.into(),
+        sp_domains::PermissionedActionAllowedBy::Anyone
+    ));
     assert_ok!(crate::Pallet::<Test>::register_domain_runtime(
         RawOrigin::Root.into(),
         "evm".to_owned(),
@@ -591,7 +594,7 @@ pub(crate) fn register_genesis_domain(creator: u128, operator_ids: Vec<OperatorI
             + <Test as pallet_balances::Config>::ExistentialDeposit::get(),
     );
     crate::Pallet::<Test>::instantiate_domain(
-        RawOrigin::Root.into(),
+        RawOrigin::Signed(creator).into(),
         DomainConfig {
             domain_name: "evm-domain".to_owned(),
             runtime_id: 0,

crates/sp-domains/src/lib.rs

@@ -784,6 +784,22 @@ impl<AccountId: Ord> OperatorAllowList<AccountId> {
     }
 }
 
+/// Permissioned actions allowed by either specific accounts or anyone.
+#[derive(TypeInfo, Encode, Decode, Debug, PartialEq, Clone, Serialize, Deserialize)]
+pub enum PermissionedActionAllowedBy<AccountId: Codec + Clone> {
+    Accounts(Vec<AccountId>),
+    Anyone,
+}
+
+impl<AccountId: Codec + PartialEq + Clone> PermissionedActionAllowedBy<AccountId> {
+    pub fn is_allowed(&self, who: &AccountId) -> bool {
+        match self {
+            PermissionedActionAllowedBy::Accounts(accounts) => accounts.contains(who),
+            PermissionedActionAllowedBy::Anyone => true,
+        }
+    }
+}
+
 #[derive(TypeInfo, Debug, Encode, Decode, Clone, PartialEq, Eq, Serialize, Deserialize)]
 pub struct GenesisDomain<AccountId: Ord, Balance> {
     // Domain runtime items
@@ -1310,9 +1326,6 @@ sp_api::decl_runtime_apis! {
         /// Get operator id by signing key
         fn operator_id_by_signing_key(signing_key: OperatorPublicKey) -> Option<OperatorId>;
 
-        /// Get the consensus chain sudo account id, currently only used in the intentional malicious operator
-        fn sudo_account_id() -> subspace_runtime_primitives::AccountId;
-
         /// Returns the execution receipt hash of the given domain and domain block number
         fn receipt_hash(domain_id: DomainId, domain_number: HeaderNumberFor<DomainHeader>) -> Option<HeaderHashFor<DomainHeader>>;
 

crates/subspace-malicious-operator/src/bin/subspace-malicious-operator.rs

@@ -93,6 +93,7 @@ fn set_default_ss58_version<C: AsRef<dyn ChainSpec>>(chain_spec: C) {
 fn main() -> Result<(), Error> {
     let cli = Cli::from_args();
 
+    let sudo_account = cli.sudo_account();
     let runner = cli.create_runner(&cli.run)?;
     set_default_ss58_version(&runner.config().chain_spec);
     runner.run_node_until_exit(|mut consensus_chain_config| async move {
@@ -354,7 +355,9 @@ fn main() -> Result<(), Error> {
                                 return;
                             }
                         };
-                        if let Err(error) = domain_starter.start(bootstrap_result).await {
+                        if let Err(error) =
+                            domain_starter.start(bootstrap_result, sudo_account).await
+                        {
                             log::error!("Domain starter exited with an error {error:?}");
                         }
                     }),

crates/subspace-malicious-operator/src/chain_spec.rs

@@ -7,7 +7,7 @@ use sc_service::{ChainSpec, ChainType, NoExtension};
 use sp_core::crypto::AccountId32;
 use sp_core::{sr25519, Pair, Public};
 use sp_domains::storage::RawGenesis;
-use sp_domains::{OperatorAllowList, OperatorPublicKey, RuntimeType};
+use sp_domains::{OperatorAllowList, OperatorPublicKey, PermissionedActionAllowedBy, RuntimeType};
 use sp_runtime::traits::{Convert, IdentifyAccount};
 use sp_runtime::{BuildStorage, MultiSigner, Percent};
 use std::marker::PhantomData;
@@ -94,6 +94,10 @@ pub fn domain_dev_config() -> GenericChainSpec<evm_domain_runtime::RuntimeGenesi
     )
 }
 
+pub(crate) fn consensus_dev_sudo_account() -> AccountId32 {
+    get_account_id_from_seed("Alice")
+}
+
 pub fn create_domain_spec(
     chain_id: &str,
     raw_genesis: RawGenesis,
@@ -151,6 +155,7 @@ struct GenesisDomainParams {
     operator_signing_key: OperatorPublicKey,
     raw_genesis_storage: Vec<u8>,
     initial_balances: Vec<(MultiAccountId, Balance)>,
+    permissioned_action_allowed_by: PermissionedActionAllowedBy<AccountId>,
 }
 
 pub fn dev_config() -> Result<GenericChainSpec<subspace_runtime::RuntimeGenesisConfig>, String> {
@@ -207,6 +212,7 @@ pub fn dev_config() -> Result<GenericChainSpec<subspace_runtime::RuntimeGenesisC
                     operator_signing_key: get_public_key_from_seed::<OperatorPublicKey>("Alice"),
                     raw_genesis_storage: raw_genesis_storage.clone(),
                     initial_balances: endowed_accounts(),
+                    permissioned_action_allowed_by: PermissionedActionAllowedBy::Anyone,
                 },
             )
         },
@@ -271,6 +277,9 @@ fn subspace_genesis_config(
             confirmation_depth_k,
         },
         domains: DomainsConfig {
+            permissioned_action_allowed_by: Some(
+                genesis_domain_params.permissioned_action_allowed_by,
+            ),
             genesis_domain: Some(sp_domains::GenesisDomain {
                 runtime_name: "evm".to_owned(),
                 runtime_type: RuntimeType::Evm,

crates/subspace-malicious-operator/src/lib.rs

@@ -29,6 +29,7 @@ use sc_cli::{
 };
 use sc_service::config::{KeystoreConfig, NetworkConfiguration};
 use sc_service::{BasePath, BlocksPruning, Configuration, DatabaseSource};
+use sp_core::crypto::{AccountId32, Ss58Codec};
 use sp_domains::DomainId;
 
 /// Subspace Cli.
@@ -43,6 +44,11 @@ pub struct Cli {
     #[clap(flatten)]
     pub run: RunCmd,
 
+    /// Sudo account to use for malicious operator
+    /// If not passed, dev sudo account is used instead.
+    #[arg(long)]
+    pub sudo_account: Option<String>,
+
     /// Domain arguments
     ///
     /// The command-line arguments provided first will be passed to the embedded consensus node,
@@ -53,6 +59,17 @@ pub struct Cli {
     pub domain_args: Vec<String>,
 }
 
+impl Cli {
+    pub fn sudo_account(&self) -> AccountId32 {
+        self.sudo_account
+            .as_ref()
+            .map(|sudo_account| {
+                AccountId32::from_ss58check(sudo_account).expect("Invalid sudo account")
+            })
+            .unwrap_or(crate::chain_spec::consensus_dev_sudo_account())
+    }
+}
+
 impl SubstrateCli for Cli {
     fn impl_name() -> String {
         "Subspace".into()

crates/subspace-malicious-operator/src/malicious_bundle_producer.rs

@@ -82,7 +82,7 @@ impl MaliciousOperatorStatus {
 
 pub struct MaliciousBundleProducer<Client, CClient, TransactionPool> {
     domain_id: DomainId,
-    sudo_acccount: AccountId,
+    sudo_account: AccountId,
     consensus_keystore: KeystorePtr,
     operator_keystore: KeystorePtr,
     consensus_client: Arc<CClient>,
@@ -118,6 +118,7 @@ where
         consensus_keystore: KeystorePtr,
         consensus_offchain_tx_pool_factory: OffchainTransactionPoolFactory<CBlock>,
         domain_transaction_pool: Arc<TransactionPool>,
+        sudo_account: AccountId,
     ) -> Self {
         let operator_keystore = KeystoreContainer::new(&KeystoreConfig::InMemory)
             .expect("create in-memory keystore container must succeed")
@@ -146,11 +147,6 @@ where
         let malicious_bundle_tamper =
             MaliciousBundleTamper::new(domain_client, operator_keystore.clone());
 
-        let sudo_acccount = consensus_client
-            .runtime_api()
-            .sudo_account_id(consensus_client.info().best_hash)
-            .expect("Failed to get sudo account");
-
         Self {
             domain_id,
             consensus_client,
@@ -159,7 +155,7 @@ where
             bundle_producer,
             malicious_bundle_tamper,
             malicious_operator_status: MaliciousOperatorStatus::NoStatus,
-            sudo_acccount,
+            sudo_account,
             consensus_offchain_tx_pool_factory,
         }
     }
@@ -317,7 +313,7 @@ where
     fn sudo_acccount_nonce(&self) -> Result<Nonce, Box<dyn Error>> {
         Ok(self.consensus_client.runtime_api().account_nonce(
             self.consensus_client.info().best_hash,
-            self.sudo_acccount.clone(),
+            self.sudo_account.clone(),
         )?)
     }
 
@@ -368,7 +364,7 @@ where
                 &self.consensus_keystore,
                 self.consensus_client.info(),
                 call.clone(),
-                self.sudo_acccount.clone(),
+                self.sudo_account.clone(),
                 nonce,
             )?,
             None => UncheckedExtrinsic::new_unsigned(call.clone()),

crates/subspace-malicious-operator/src/malicious_domain_instance_starter.rs

@@ -23,6 +23,7 @@ use std::path::PathBuf;
 use std::sync::Arc;
 use subspace_runtime::RuntimeApi as CRuntimeApi;
 use subspace_runtime_primitives::opaque::Block as CBlock;
+use subspace_runtime_primitives::AccountId;
 use subspace_service::FullClient as CFullClient;
 
 /// `DomainInstanceStarter` used to start a domain instance node based on the given
@@ -51,6 +52,7 @@ where
     pub async fn start(
         self,
         bootstrap_result: BootstrapResult<CBlock>,
+        sudo_account: AccountId,
     ) -> std::result::Result<(), Box<dyn std::error::Error>> {
         let BootstrapResult {
             domain_instance_data,
@@ -178,6 +180,7 @@ where
                     consensus_keystore,
                     consensus_offchain_tx_pool_factory,
                     domain_node.transaction_pool.clone(),
+                    sudo_account,
                 );
 
                 domain_node