Add Registrar proxy type (#358)

* registrar proxy

* typescript api

* wip test

* simplify test

* proxy sudo registar

* wip test for sudo registrar proxy

* wip

* fix tests

* allow utility call for all proxies (with non privilege escalation test)

* wip fix test after merge

* apply proxy utility change to all runtimes + fmt

* Add DataPreservers calls to filter

Co-authored-by: tmpolaczyk <44604217+tmpolaczyk@users.noreply.github.com>

* fmt

* Add DataPreservers calls to Registar proxy

Co-authored-by: tmpolaczyk <44604217+tmpolaczyk@users.noreply.github.com>

* lint:fix

* fmt

* improve test

* fix sudo key set it

---------

Co-authored-by: girazoki <gorka.irazoki@gmail.com>
Co-authored-by: tmpolaczyk <44604217+tmpolaczyk@users.noreply.github.com>

container-chains/templates/frontier/node/src/rpc/mod.rs

@@ -23,11 +23,9 @@
 
 pub use sc_rpc::{DenyUnsafe, SubscriptionTaskExecutor};
 
-use sp_consensus_aura::SlotDuration;
 use {
     container_chain_template_frontier_runtime::{opaque::Block, AccountId, Hash, Index},
-    cumulus_primitives_core::ParaId,
-    cumulus_primitives_core::PersistedValidationData,
+    cumulus_primitives_core::{ParaId, PersistedValidationData},
     cumulus_primitives_parachain_inherent::ParachainInherentData,
     cumulus_test_relay_sproof_builder::RelayStateSproofBuilder,
     fc_rpc::{EthTask, TxPool},
@@ -52,6 +50,7 @@ use {
     sp_blockchain::{
         Backend as BlockchainBackend, Error as BlockChainError, HeaderBackend, HeaderMetadata,
     },
+    sp_consensus_aura::SlotDuration,
     sp_core::H256,
     sp_runtime::traits::{BlakeTwo256, Block as BlockT},
     std::{sync::Arc, time::Duration},

container-chains/templates/frontier/runtime/src/lib.rs

@@ -563,6 +563,12 @@ impl Default for ProxyType {
 
 impl InstanceFilter<RuntimeCall> for ProxyType {
     fn filter(&self, c: &RuntimeCall) -> bool {
+        // Since proxy filters are respected in all dispatches of the Utility
+        // pallet, it should never need to be filtered by any proxy.
+        if let RuntimeCall::Utility(..) = c {
+            return true;
+        }
+
         match self {
             ProxyType::Any => true,
             ProxyType::NonTransfer => {
@@ -571,17 +577,17 @@ impl InstanceFilter<RuntimeCall> for ProxyType {
                     RuntimeCall::System(..)
                         | RuntimeCall::ParachainSystem(..)
                         | RuntimeCall::Timestamp(..)
-                        | RuntimeCall::Utility(..)
                         | RuntimeCall::Proxy(..)
                 )
             }
-            ProxyType::Governance => matches!(c, RuntimeCall::Utility(..)),
+            // We don't have governance yet
+            ProxyType::Governance => false,
             ProxyType::CancelProxy => matches!(
                 c,
                 RuntimeCall::Proxy(pallet_proxy::Call::reject_announcement { .. })
             ),
             ProxyType::Balances => {
-                matches!(c, RuntimeCall::Balances(..) | RuntimeCall::Utility(..))
+                matches!(c, RuntimeCall::Balances(..))
             }
         }
     }

container-chains/templates/simple/runtime/src/lib.rs

@@ -449,6 +449,12 @@ impl Default for ProxyType {
 
 impl InstanceFilter<RuntimeCall> for ProxyType {
     fn filter(&self, c: &RuntimeCall) -> bool {
+        // Since proxy filters are respected in all dispatches of the Utility
+        // pallet, it should never need to be filtered by any proxy.
+        if let RuntimeCall::Utility(..) = c {
+            return true;
+        }
+
         match self {
             ProxyType::Any => true,
             ProxyType::NonTransfer => {
@@ -457,17 +463,17 @@ impl InstanceFilter<RuntimeCall> for ProxyType {
                     RuntimeCall::System(..)
                         | RuntimeCall::ParachainSystem(..)
                         | RuntimeCall::Timestamp(..)
-                        | RuntimeCall::Utility(..)
                         | RuntimeCall::Proxy(..)
                 )
             }
-            ProxyType::Governance => matches!(c, RuntimeCall::Utility(..)),
+            // We don't have governance yet
+            ProxyType::Governance => false,
             ProxyType::CancelProxy => matches!(
                 c,
                 RuntimeCall::Proxy(pallet_proxy::Call::reject_announcement { .. })
             ),
             ProxyType::Balances => {
-                matches!(c, RuntimeCall::Balances(..) | RuntimeCall::Utility(..))
+                matches!(c, RuntimeCall::Balances(..))
             }
         }
     }

pallets/registrar/src/lib.rs

@@ -251,8 +251,7 @@ pub mod pallet {
     impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
         #[cfg(feature = "try-runtime")]
         fn try_state(_n: BlockNumberFor<T>) -> Result<(), sp_runtime::TryRuntimeError> {
-            use scale_info::prelude::format;
-            use sp_std::collections::btree_set::BTreeSet;
+            use {scale_info::prelude::format, sp_std::collections::btree_set::BTreeSet};
             // A para id can only be in 1 of [`RegisteredParaIds`, `PendingVerification`, `Paused`]
             // Get all those para ids and check for duplicates
             let mut para_ids: Vec<ParaId> = vec![];
@@ -604,10 +603,10 @@ pub mod pallet {
 
         #[cfg(feature = "runtime-benchmarks")]
         pub fn benchmarks_get_or_create_para_manager(para_id: &ParaId) -> Result<T::AccountId, ()> {
-            use frame_benchmarking::account;
-            use frame_support::assert_ok;
-            use frame_support::dispatch::RawOrigin;
-            use frame_support::traits::Currency;
+            use {
+                frame_benchmarking::account,
+                frame_support::{assert_ok, dispatch::RawOrigin, traits::Currency},
+            };
             // Return container chain manager, or register container chain as ALICE if it does not exist
             if !ParaGenesisData::<T>::contains_key(para_id) {
                 // Register as a new user

runtime/dancebox/src/lib.rs

@@ -36,7 +36,6 @@ pub mod weights;
 #[cfg(feature = "try-runtime")]
 use sp_runtime::TryRuntimeError;
 
-use frame_support::traits::EitherOfDiverse;
 use {
     cumulus_pallet_parachain_system::{RelayChainStateProof, RelayNumberStrictlyIncreases},
     cumulus_primitives_core::{
@@ -50,8 +49,8 @@ use {
         parameter_types,
         traits::{
             fungible::{Balanced, Credit},
-            ConstU128, ConstU32, ConstU64, ConstU8, Contains, InsideBoth, InstanceFilter,
-            OffchainWorker, OnFinalize, OnIdle, OnInitialize, OnRuntimeUpgrade,
+            ConstU128, ConstU32, ConstU64, ConstU8, Contains, EitherOfDiverse, InsideBoth,
+            InstanceFilter, OffchainWorker, OnFinalize, OnIdle, OnInitialize, OnRuntimeUpgrade,
             ValidatorRegistration,
         },
         weights::{
@@ -937,6 +936,10 @@ pub enum ProxyType {
     CancelProxy = 4,
     /// Allow extrinsic related to Balances.
     Balances = 5,
+    /// Allow extrinsics related to Registrar
+    Registrar = 6,
+    /// Allow extrinsics related to Registrar that needs to be called through Sudo
+    SudoRegistrar = 7,
 }
 
 impl Default for ProxyType {
@@ -947,6 +950,12 @@ impl Default for ProxyType {
 
 impl InstanceFilter<RuntimeCall> for ProxyType {
     fn filter(&self, c: &RuntimeCall) -> bool {
+        // Since proxy filters are respected in all dispatches of the Utility
+        // pallet, it should never need to be filtered by any proxy.
+        if let RuntimeCall::Utility(..) = c {
+            return true;
+        }
+
         match self {
             ProxyType::Any => true,
             ProxyType::NonTransfer => {
@@ -955,25 +964,37 @@ impl InstanceFilter<RuntimeCall> for ProxyType {
                     RuntimeCall::System(..)
                         | RuntimeCall::ParachainSystem(..)
                         | RuntimeCall::Timestamp(..)
-                        | RuntimeCall::Utility(..)
                         | RuntimeCall::Proxy(..)
                         | RuntimeCall::Registrar(..)
                 )
             }
-            ProxyType::Governance => matches!(c, RuntimeCall::Utility(..)),
-            ProxyType::Staking => matches!(
-                c,
-                RuntimeCall::Session(..)
-                    | RuntimeCall::Utility(..)
-                    | RuntimeCall::PooledStaking(..)
-            ),
+            // We don't have governance yet
+            ProxyType::Governance => false,
+            ProxyType::Staking => {
+                matches!(c, RuntimeCall::Session(..) | RuntimeCall::PooledStaking(..))
+            }
             ProxyType::CancelProxy => matches!(
                 c,
                 RuntimeCall::Proxy(pallet_proxy::Call::reject_announcement { .. })
             ),
             ProxyType::Balances => {
-                matches!(c, RuntimeCall::Balances(..) | RuntimeCall::Utility(..))
+                matches!(c, RuntimeCall::Balances(..))
             }
+            ProxyType::Registrar => {
+                matches!(
+                    c,
+                    RuntimeCall::Registrar(..) | RuntimeCall::DataPreservers(..)
+                )
+            }
+            ProxyType::SudoRegistrar => match c {
+                RuntimeCall::Sudo(pallet_sudo::Call::sudo { call: ref x }) => {
+                    matches!(
+                        x.as_ref(),
+                        &RuntimeCall::Registrar(..) | &RuntimeCall::DataPreservers(..)
+                    )
+                }
+                _ => false,
+            },
         }
     }
 

runtime/dancebox/src/migrations.rs

@@ -22,8 +22,12 @@
 use {
     crate::{Invulnerables, ParaId, Runtime, RuntimeOrigin, ServicesPayment, LOG_TARGET},
     frame_support::{
-        migration::storage_key_iter, pallet_prelude::ValueQuery, storage::types::StorageMap,
-        storage::types::StorageValue, traits::OnRuntimeUpgrade, weights::Weight, Blake2_128Concat,
+        migration::storage_key_iter,
+        pallet_prelude::ValueQuery,
+        storage::types::{StorageMap, StorageValue},
+        traits::OnRuntimeUpgrade,
+        weights::Weight,
+        Blake2_128Concat,
     },
     pallet_balances::IdAmount,
     pallet_configuration::{weights::WeightInfo as _, HostConfiguration},

runtime/dancebox/tests/common/mod.rs

@@ -201,6 +201,8 @@ pub struct ExtBuilder {
     balances: Vec<(AccountId, Balance)>,
     // [collator, amount]
     collators: Vec<(AccountId, Balance)>,
+    // sudo key
+    sudo: Option<AccountId>,
     // list of registered para ids: para_id, genesis_data, boot_nodes, block_credits
     para_ids: Vec<(
         u32,
@@ -225,6 +227,11 @@ impl ExtBuilder {
         self
     }
 
+    pub fn with_sudo(mut self, sudo: AccountId) -> Self {
+        self.sudo = Some(sudo);
+        self
+    }
+
     pub fn with_para_ids(
         mut self,
         para_ids: Vec<(
@@ -347,6 +354,10 @@ impl ExtBuilder {
                 .assimilate_storage(&mut t)
                 .unwrap();
         }
+        pallet_sudo::GenesisConfig::<Runtime> { key: self.sudo }
+            .assimilate_storage(&mut t)
+            .unwrap();
+
         t
     }
 

runtime/dancebox/tests/integration_test.rs

@@ -2307,6 +2307,86 @@ fn test_proxy_non_transfer() {
         });
 }
 
+#[test]
+fn test_proxy_utility() {
+    // All proxy types should be able to use Utility pallet, but we ensure
+    // subcalls don't allow to circumvent filters.
+
+    // Dummy match to ensure we update this test when adding new proxy types.
+    match ProxyType::Any {
+        ProxyType::Any
+        | ProxyType::NonTransfer
+        | ProxyType::Governance
+        | ProxyType::Staking
+        | ProxyType::CancelProxy
+        | ProxyType::Balances
+        | ProxyType::Registrar
+        | ProxyType::SudoRegistrar => (),
+    };
+
+    // All except for any
+    let proxy_types = &[
+        ProxyType::NonTransfer,
+        ProxyType::Governance,
+        ProxyType::Staking,
+        ProxyType::CancelProxy,
+        ProxyType::Balances,
+        ProxyType::Registrar,
+        ProxyType::SudoRegistrar,
+    ];
+
+    for &proxy_type in proxy_types {
+        ExtBuilder::default()
+            .with_balances(vec![
+                // Alice gets 10k extra tokens for her mapping deposit
+                (AccountId::from(ALICE), 210_000 * UNIT),
+                (AccountId::from(BOB), 100_000 * UNIT),
+                (AccountId::from(CHARLIE), 100_000 * UNIT),
+                (AccountId::from(DAVE), 100_000 * UNIT),
+            ])
+            .with_collators(vec![
+                (AccountId::from(ALICE), 210 * UNIT),
+                (AccountId::from(BOB), 100 * UNIT),
+            ])
+            .with_sudo(AccountId::from(ALICE))
+            .with_config(default_config())
+            .build()
+            .execute_with(|| {
+                assert_ok!(Proxy::add_proxy(
+                    origin_of(ALICE.into()),
+                    AccountId::from(BOB).into(),
+                    proxy_type,
+                    0
+                ));
+
+                let free_balance = Balances::free_balance(AccountId::from(BOB));
+
+                assert_ok!(Proxy::proxy(
+                    origin_of(BOB.into()),
+                    AccountId::from(ALICE).into(),
+                    None,
+                    Box::new(
+                        pallet_sudo::Call::sudo {
+                            call: Box::new(
+                                pallet_utility::Call::batch {
+                                    calls: vec![pallet_balances::Call::force_set_balance {
+                                        who: AccountId::from(BOB).into(),
+                                        new_free: 42424242424242
+                                    }
+                                    .into()]
+                                }
+                                .into()
+                            )
+                        }
+                        .into()
+                    )
+                ));
+
+                assert_eq!(Balances::free_balance(AccountId::from(BOB)), free_balance);
+            });
+    }
+}
+
 #[test]
 fn check_well_known_keys() {
     use frame_support::traits::PalletInfo;