Skip to content
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.

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

Add Registrar proxy type #358

Merged
merged 22 commits into from
Dec 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 4 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 2 additions & 3 deletions container-chains/templates/frontier/node/src/rpc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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},
Expand Down
12 changes: 9 additions & 3 deletions container-chains/templates/frontier/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand All @@ -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(..))
}
}
}
Expand Down
12 changes: 9 additions & 3 deletions container-chains/templates/simple/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand All @@ -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(..))
}
}
}
Expand Down
11 changes: 5 additions & 6 deletions pallets/registrar/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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![];
Expand Down Expand Up @@ -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
Expand Down
45 changes: 33 additions & 12 deletions runtime/dancebox/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand All @@ -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::{
Expand Down Expand Up @@ -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 {
Expand All @@ -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;
}
tmpolaczyk marked this conversation as resolved.
Show resolved Hide resolved

match self {
ProxyType::Any => true,
ProxyType::NonTransfer => {
Expand All @@ -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 => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we also add a proxy type called SUdoRegistrar that is sable to call with sudo any of the registrar calls? similar to https://github.com/paritytech/polkadot-sdk/blob/e7651cf41b2d775dedcc897e17ead16e200a4fff/polkadot/runtime/westend/src/lib.rs#L1064

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,
},
}
}

Expand Down
8 changes: 6 additions & 2 deletions runtime/dancebox/src/migrations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
11 changes: 11 additions & 0 deletions runtime/dancebox/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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<(
Expand Down Expand Up @@ -347,6 +354,10 @@ impl ExtBuilder {
.assimilate_storage(&mut t)
.unwrap();
}
pallet_sudo::GenesisConfig::<Runtime> { key: self.sudo }
.assimilate_storage(&mut t)
.unwrap();

t
}

Expand Down
80 changes: 80 additions & 0 deletions runtime/dancebox/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
];
Copy link
Contributor

Choose a reason for hiding this comment

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

There are some external crates that allow iterating over enums, not sure if we already have one, but as a nice hack we could use the Decode trait (not tested but should work):

for x in 0u8.. {
    if let Ok(x) = x.decode::<ProxyType>() {
        proxy_types.push(x);
    } else {
        break;
    }
}


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;
Expand Down
Loading
Loading