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.

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

Jump to bottom

Add Registrar proxy type #358

Merged
merged 22 commits into from
Dec 19, 2023
Merged

Add Registrar proxy type #358

Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
547c918
registrar proxy
nanocryk Dec 1, 2023
5f9deee
Merge remote-tracking branch 'origin/master' into jeremy-registar-pro…
nanocryk Dec 4, 2023
3c3ab73
typescript api
nanocryk Dec 4, 2023
bd9ad7a
wip test
nanocryk Dec 6, 2023
bf6add5
simplify test
girazoki Dec 6, 2023
0fe62d1
proxy sudo registar
nanocryk Dec 7, 2023
2b0ced8
wip test for sudo registrar proxy
nanocryk Dec 11, 2023
58d5901
wip
nanocryk Dec 12, 2023
75389dd
fix tests
nanocryk Dec 12, 2023
2ed805d
allow utility call for all proxies (with non privilege escalation test)
nanocryk Dec 14, 2023
4670784
Merge remote-tracking branch 'origin/master' into jeremy-registar-pro…
nanocryk Dec 14, 2023
ab9ebca
wip fix test after merge
nanocryk Dec 14, 2023
d2e3b73
apply proxy utility change to all runtimes + fmt
nanocryk Dec 15, 2023
0e835ac
Merge remote-tracking branch 'origin/master' into jeremy-registar-pro…
nanocryk Dec 15, 2023
188d93b
Add DataPreservers calls to filter
nanocryk Dec 18, 2023
ae69abe
Merge remote-tracking branch 'origin/master' into jeremy-registar-pro…
nanocryk Dec 18, 2023
2de2437
fmt
nanocryk Dec 18, 2023
3232944
Add DataPreservers calls to Registar proxy
nanocryk Dec 18, 2023
c4927e2
lint:fix
nanocryk Dec 18, 2023
d10da31
fmt
nanocryk Dec 19, 2023
383761c
improve test
nanocryk Dec 19, 2023
c3114c5
fix sudo key set it
girazoki Dec 19, 2023
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
View file
Open in desktop

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
74 changes: 74 additions & 0 deletions runtime/dancebox/tests/integration_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2307,6 +2307,80 @@ 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 => (),
};

let proxy_types = &[
ProxyType::Any,
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_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_utility::Call::batch {
calls: vec![pallet_balances::Call::force_set_balance {
Copy link
Contributor

Choose a reason for hiding this comment

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

force_set_balance will always fail because it needs root origin, maybe this test is missing a call to sudo first?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes the test needs to be batch->sudo.>force_set_balacne

Copy link
Contributor

Choose a reason for hiding this comment

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

But ProxyType::Any should allow this, right? Then why doesn't the test fail?

Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm that's a good question

Copy link
Collaborator

Choose a reason for hiding this comment

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

its probably becaues we are still not going through the sudo pallet right?

who: AccountId::from(BOB).into(),
new_free: 42424242424242
}
.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