From f73d4e5974b52e8f3f97df0c30c18274da425c49 Mon Sep 17 00:00:00 2001 From: Alejandro Martinez Andres <11448715+al3mart@users.noreply.github.com> Date: Tue, 21 Jan 2025 01:11:20 +0100 Subject: [PATCH 1/9] refactor(mainnet): pallet_proxy config & tests --- runtime/mainnet/src/config/proxy.rs | 104 ++++++++++++++++++++++++++-- 1 file changed, 98 insertions(+), 6 deletions(-) diff --git a/runtime/mainnet/src/config/proxy.rs b/runtime/mainnet/src/config/proxy.rs index 0b8ab9b99..03907af35 100644 --- a/runtime/mainnet/src/config/proxy.rs +++ b/runtime/mainnet/src/config/proxy.rs @@ -1,11 +1,9 @@ use frame_support::traits::InstanceFilter; -use pop_runtime_common::proxy::{ - AnnouncementDepositBase, AnnouncementDepositFactor, MaxPending, MaxProxies, ProxyDepositBase, - ProxyDepositFactor, ProxyType, -}; -use sp_runtime::traits::BlakeTwo256; +use pop_runtime_common::proxy::{MaxPending, MaxProxies, ProxyType}; -use crate::{Balances, Runtime, RuntimeCall, RuntimeEvent}; +use crate::{ + deposit, parameter_types, Balance, Balances, BlakeTwo256, Runtime, RuntimeCall, RuntimeEvent, +}; impl InstanceFilter for ProxyType { fn filter(&self, c: &RuntimeCall) -> bool { @@ -41,6 +39,17 @@ impl InstanceFilter for ProxyType { } } +parameter_types! { + // One storage item; key size 32, value size 16. + pub const ProxyDepositBase: Balance = deposit(1, 48); + // Additional storage item size of AccountId 32 bytes + ProxyType 1 byte + BlockNum 4 bytes. + pub const ProxyDepositFactor: Balance = deposit(0, 37); + // One storage item; key size 32, value size 16. + pub const AnnouncementDepositBase: Balance = deposit(1, 48); + // Additional storage item 32 bytes AccountId + 32 bytes Hash + 4 bytes BlockNum. + pub const AnnouncementDepositFactor: Balance = deposit(0, 68); +} + impl pallet_proxy::Config for Runtime { type AnnouncementDepositBase = AnnouncementDepositBase; type AnnouncementDepositFactor = AnnouncementDepositFactor; @@ -55,3 +64,86 @@ impl pallet_proxy::Config for Runtime { type RuntimeEvent = RuntimeEvent; type WeightInfo = pallet_proxy::weights::SubstrateWeight; } + +#[cfg(test)] +mod tests { + use std::any::TypeId; + + use frame_support::traits::Get; + + use super::*; + + #[test] + fn proxy_does_not_use_default_weights() { + assert_ne!( + TypeId::of::<::WeightInfo>(), + TypeId::of::<()>(), + ); + } + + #[test] + fn pallet_proxy_uses_proxy_type() { + assert_eq!( + TypeId::of::<::ProxyType>(), + TypeId::of::(), + ); + } + + #[test] + fn proxy_has_deposit_factor() { + assert_eq!( + <::ProxyDepositFactor as Get>::get(), + deposit(0, 37), + ); + } + + #[test] + fn proxy_has_deposit_base() { + assert_eq!( + <::ProxyDepositBase as Get>::get(), + deposit(1, 48), + ); + } + + #[test] + fn proxy_uses_balances_as_currency() { + assert_eq!( + TypeId::of::<::Currency>(), + TypeId::of::(), + ); + } + + #[test] + fn proxy_configures_max_num_of_proxies() { + assert_eq!(<::MaxProxies>::get(), 32,); + } + + #[test] + fn proxy_configures_max_pending() { + assert_eq!(<::MaxPending>::get(), 32,); + } + + #[test] + fn proxy_uses_blaketwo256_as_hasher() { + assert_eq!( + TypeId::of::<::CallHasher>(), + TypeId::of::(), + ); + } + + #[test] + fn proxy_has_announcement_deposit_factor() { + assert_eq!( + <::AnnouncementDepositFactor as Get>::get(), + deposit(0, 68), + ); + } + + #[test] + fn proxy_has_announcement_deposit_base() { + assert_eq!( + <::AnnouncementDepositBase as Get>::get(), + deposit(1, 48), + ); + } +} From cc13db137df2c2201e2a465d6a58838014f2db55 Mon Sep 17 00:00:00 2001 From: Alejandro Martinez Andres <11448715+al3mart@users.noreply.github.com> Date: Mon, 27 Jan 2025 18:53:45 +0100 Subject: [PATCH 2/9] style(proxy): tests follow config order --- runtime/mainnet/src/config/proxy.rs | 59 ++++++++++++++--------------- 1 file changed, 29 insertions(+), 30 deletions(-) diff --git a/runtime/mainnet/src/config/proxy.rs b/runtime/mainnet/src/config/proxy.rs index 03907af35..0e267766b 100644 --- a/runtime/mainnet/src/config/proxy.rs +++ b/runtime/mainnet/src/config/proxy.rs @@ -74,34 +74,25 @@ mod tests { use super::*; #[test] - fn proxy_does_not_use_default_weights() { - assert_ne!( - TypeId::of::<::WeightInfo>(), - TypeId::of::<()>(), - ); - } - - #[test] - fn pallet_proxy_uses_proxy_type() { + fn proxy_has_announcement_deposit_base() { assert_eq!( - TypeId::of::<::ProxyType>(), - TypeId::of::(), + <::AnnouncementDepositBase as Get>::get(), + deposit(1, 48), ); } - #[test] - fn proxy_has_deposit_factor() { + fn proxy_has_announcement_deposit_factor() { assert_eq!( - <::ProxyDepositFactor as Get>::get(), - deposit(0, 37), + <::AnnouncementDepositFactor as Get>::get(), + deposit(0, 68), ); } #[test] - fn proxy_has_deposit_base() { + fn proxy_uses_blaketwo256_as_hasher() { assert_eq!( - <::ProxyDepositBase as Get>::get(), - deposit(1, 48), + TypeId::of::<::CallHasher>(), + TypeId::of::(), ); } @@ -113,37 +104,45 @@ mod tests { ); } + #[test] + fn proxy_configures_max_pending() { + assert_eq!(<::MaxPending>::get(), 32,); + } + #[test] fn proxy_configures_max_num_of_proxies() { assert_eq!(<::MaxProxies>::get(), 32,); } #[test] - fn proxy_configures_max_pending() { - assert_eq!(<::MaxPending>::get(), 32,); + fn proxy_has_deposit_base() { + assert_eq!( + <::ProxyDepositBase as Get>::get(), + deposit(1, 48), + ); } #[test] - fn proxy_uses_blaketwo256_as_hasher() { + fn proxy_has_deposit_factor() { assert_eq!( - TypeId::of::<::CallHasher>(), - TypeId::of::(), + <::ProxyDepositFactor as Get>::get(), + deposit(0, 37), ); } #[test] - fn proxy_has_announcement_deposit_factor() { + fn pallet_proxy_uses_proxy_type() { assert_eq!( - <::AnnouncementDepositFactor as Get>::get(), - deposit(0, 68), + TypeId::of::<::ProxyType>(), + TypeId::of::(), ); } #[test] - fn proxy_has_announcement_deposit_base() { - assert_eq!( - <::AnnouncementDepositBase as Get>::get(), - deposit(1, 48), + fn proxy_does_not_use_default_weights() { + assert_ne!( + TypeId::of::<::WeightInfo>(), + TypeId::of::<()>(), ); } } From d3bfc6c748aa46e487980649bfaaa9455ffc70b0 Mon Sep 17 00:00:00 2001 From: Alejandro Martinez Andres <11448715+al3mart@users.noreply.github.com> Date: Wed, 29 Jan 2025 18:39:27 +0100 Subject: [PATCH 3/9] refactor(proxy): remove asset related ProxyTypes --- runtime/common/src/lib.rs | 8 -------- runtime/mainnet/src/config/proxy.rs | 9 --------- 2 files changed, 17 deletions(-) diff --git a/runtime/common/src/lib.rs b/runtime/common/src/lib.rs index e0890ee3f..e0d73b910 100644 --- a/runtime/common/src/lib.rs +++ b/runtime/common/src/lib.rs @@ -116,12 +116,6 @@ pub mod proxy { NonTransfer, /// Proxy with the ability to reject time-delay proxy announcements. CancelProxy, - /// Assets proxy. Can execute any call from `assets`, **including asset transfers**. - Assets, - /// Owner proxy. Can execute calls related to asset ownership. - AssetOwner, - /// Asset manager. Can execute calls related to asset management. - AssetManager, /// Collator selection proxy. Can execute calls related to collator selection mechanism. Collator, } @@ -137,8 +131,6 @@ pub mod proxy { (x, y) if x == y => true, (ProxyType::Any, _) => true, (_, ProxyType::Any) => false, - (ProxyType::Assets, ProxyType::AssetOwner) => true, - (ProxyType::Assets, ProxyType::AssetManager) => true, (ProxyType::NonTransfer, ProxyType::Collator) => true, _ => false, } diff --git a/runtime/mainnet/src/config/proxy.rs b/runtime/mainnet/src/config/proxy.rs index 0e267766b..a54b8f1f5 100644 --- a/runtime/mainnet/src/config/proxy.rs +++ b/runtime/mainnet/src/config/proxy.rs @@ -16,15 +16,6 @@ impl InstanceFilter for ProxyType { RuntimeCall::Utility { .. } | RuntimeCall::Multisig { .. } ), - ProxyType::Assets => { - matches!(c, RuntimeCall::Utility { .. } | RuntimeCall::Multisig { .. }) - }, - ProxyType::AssetOwner => { - matches!(c, RuntimeCall::Utility { .. } | RuntimeCall::Multisig { .. }) - }, - ProxyType::AssetManager => { - matches!(c, RuntimeCall::Utility { .. } | RuntimeCall::Multisig { .. }) - }, ProxyType::Collator => matches!( c, RuntimeCall::CollatorSelection { .. } | From d4c936e5bca5659e6ed1324b4156ab6f82e1866e Mon Sep 17 00:00:00 2001 From: Alejandro Martinez Andres <11448715+al3mart@users.noreply.github.com> Date: Wed, 29 Jan 2025 19:09:42 +0100 Subject: [PATCH 4/9] refactor(proxy): update ProxyDepositBase price --- runtime/mainnet/src/config/proxy.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime/mainnet/src/config/proxy.rs b/runtime/mainnet/src/config/proxy.rs index a54b8f1f5..27dbb803c 100644 --- a/runtime/mainnet/src/config/proxy.rs +++ b/runtime/mainnet/src/config/proxy.rs @@ -31,8 +31,8 @@ impl InstanceFilter for ProxyType { } parameter_types! { - // One storage item; key size 32, value size 16. - pub const ProxyDepositBase: Balance = deposit(1, 48); + // One storage item; key size 32 + hash size 8. + pub const ProxyDepositBase: Balance = deposit(1, 40); // Additional storage item size of AccountId 32 bytes + ProxyType 1 byte + BlockNum 4 bytes. pub const ProxyDepositFactor: Balance = deposit(0, 37); // One storage item; key size 32, value size 16. From cd7ebb289b78b1a67d8a21b66090ca57280c2cc2 Mon Sep 17 00:00:00 2001 From: Alejandro Martinez Andres <11448715+al3mart@users.noreply.github.com> Date: Wed, 29 Jan 2025 19:44:37 +0100 Subject: [PATCH 5/9] refactor(proxy): revert d3bfc6c and redefine ProxyType for mainnet typo --- runtime/common/src/lib.rs | 8 +++ runtime/mainnet/src/config/proxy.rs | 77 ++++++++++++++++++++++++++++- 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/runtime/common/src/lib.rs b/runtime/common/src/lib.rs index e0d73b910..e0890ee3f 100644 --- a/runtime/common/src/lib.rs +++ b/runtime/common/src/lib.rs @@ -116,6 +116,12 @@ pub mod proxy { NonTransfer, /// Proxy with the ability to reject time-delay proxy announcements. CancelProxy, + /// Assets proxy. Can execute any call from `assets`, **including asset transfers**. + Assets, + /// Owner proxy. Can execute calls related to asset ownership. + AssetOwner, + /// Asset manager. Can execute calls related to asset management. + AssetManager, /// Collator selection proxy. Can execute calls related to collator selection mechanism. Collator, } @@ -131,6 +137,8 @@ pub mod proxy { (x, y) if x == y => true, (ProxyType::Any, _) => true, (_, ProxyType::Any) => false, + (ProxyType::Assets, ProxyType::AssetOwner) => true, + (ProxyType::Assets, ProxyType::AssetManager) => true, (ProxyType::NonTransfer, ProxyType::Collator) => true, _ => false, } diff --git a/runtime/mainnet/src/config/proxy.rs b/runtime/mainnet/src/config/proxy.rs index 27dbb803c..607ed91e1 100644 --- a/runtime/mainnet/src/config/proxy.rs +++ b/runtime/mainnet/src/config/proxy.rs @@ -1,10 +1,54 @@ +use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::traits::InstanceFilter; -use pop_runtime_common::proxy::{MaxPending, MaxProxies, ProxyType}; +use pop_runtime_common::proxy::{MaxPending, MaxProxies}; +use sp_runtime::RuntimeDebug; use crate::{ deposit, parameter_types, Balance, Balances, BlakeTwo256, Runtime, RuntimeCall, RuntimeEvent, }; +/// The type used to represent the kinds of proxying allowed. +#[derive( + Copy, + Clone, + Eq, + PartialEq, + Ord, + PartialOrd, + Encode, + Decode, + RuntimeDebug, + MaxEncodedLen, + scale_info::TypeInfo, +)] +pub enum ProxyType { + /// Fully permissioned proxy. Can execute any call on behalf of _proxied_. + Any, + /// Can execute any call that does not transfer funds or assets. + NonTransfer, + /// Proxy with the ability to reject time-delay proxy announcements. + CancelProxy, + /// Collator selection proxy. Can execute calls related to collator selection mechanism. + Collator, +} +impl Default for ProxyType { + fn default() -> Self { + Self::Any + } +} + +impl ProxyType { + pub fn is_superset(s: &ProxyType, o: &ProxyType) -> bool { + match (s, o) { + (x, y) if x == y => true, + (ProxyType::Any, _) => true, + (_, ProxyType::Any) => false, + (ProxyType::NonTransfer, ProxyType::Collator) => true, + _ => false, + } + } +} + impl InstanceFilter for ProxyType { fn filter(&self, c: &RuntimeCall) -> bool { match self { @@ -64,6 +108,37 @@ mod tests { use super::*; + #[test] + fn proxy_type_default_is_any() { + assert_eq!(ProxyType::default(), ProxyType::Any); + } + + #[test] + fn proxy_type_superset_as_defined() { + let all_proxies = vec![ + ProxyType::Any, + ProxyType::NonTransfer, + ProxyType::CancelProxy, + ProxyType::Collator, + ]; + for proxy in all_proxies { + // Every proxy is part of itself. + assert!(ProxyType::is_superset(&proxy, &proxy)); + + // Any contains all others, but is not contained. + if proxy != ProxyType::Any { + assert!(ProxyType::is_superset(&ProxyType::Any, &proxy)); + assert!(!ProxyType::is_superset(&proxy, &ProxyType::Any)); + } + // CancelProxy does not contain any other proxy. + if proxy != ProxyType::CancelProxy { + assert!(!ProxyType::is_superset(&ProxyType::CancelProxy, &proxy)); + } + } + assert!(ProxyType::is_superset(&ProxyType::NonTransfer, &ProxyType::Collator)); + assert!(!ProxyType::is_superset(&ProxyType::Collator, &ProxyType::NonTransfer)); + } + #[test] fn proxy_has_announcement_deposit_base() { assert_eq!( From 4940461b6ce1912369409288adaee17744293ecc Mon Sep 17 00:00:00 2001 From: Alejandro Martinez Andres <11448715+al3mart@users.noreply.github.com> Date: Thu, 30 Jan 2025 08:20:01 +0100 Subject: [PATCH 6/9] fix(proxy): amend proxy_has_deposit_base test --- runtime/mainnet/src/config/proxy.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/mainnet/src/config/proxy.rs b/runtime/mainnet/src/config/proxy.rs index 607ed91e1..cb9675ed5 100644 --- a/runtime/mainnet/src/config/proxy.rs +++ b/runtime/mainnet/src/config/proxy.rs @@ -184,7 +184,7 @@ mod tests { fn proxy_has_deposit_base() { assert_eq!( <::ProxyDepositBase as Get>::get(), - deposit(1, 48), + deposit(1, 40), ); } From 9f9a4de6c77e7988d836a54415576b1e2717569b Mon Sep 17 00:00:00 2001 From: Alejandro Martinez Andres <11448715+al3mart@users.noreply.github.com> Date: Mon, 3 Feb 2025 13:29:17 +0100 Subject: [PATCH 7/9] docs(proxy): Clarify new ProxyType definition --- runtime/mainnet/src/config/proxy.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/runtime/mainnet/src/config/proxy.rs b/runtime/mainnet/src/config/proxy.rs index cb9675ed5..7db42c546 100644 --- a/runtime/mainnet/src/config/proxy.rs +++ b/runtime/mainnet/src/config/proxy.rs @@ -8,6 +8,10 @@ use crate::{ }; /// The type used to represent the kinds of proxying allowed. +// Mainnet will use this definition of ProxyType instead of the ones in +// `pop-common` crates until `pallet-assets` is integrated in the runtime. +// `ProxyType` in `pop-common` include Assets specific proxies which won't +// make much sense in this runtime. #[derive( Copy, Clone, From 0f714aaddb2a1bd989b910aaa0781579d0bfd82a Mon Sep 17 00:00:00 2001 From: Alejandro Martinez Andres <11448715+al3mart@users.noreply.github.com> Date: Mon, 3 Feb 2025 13:39:55 +0100 Subject: [PATCH 8/9] docs(proxy): add is_superset comments --- runtime/mainnet/src/config/proxy.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/runtime/mainnet/src/config/proxy.rs b/runtime/mainnet/src/config/proxy.rs index 7db42c546..1bb7c40e1 100644 --- a/runtime/mainnet/src/config/proxy.rs +++ b/runtime/mainnet/src/config/proxy.rs @@ -42,6 +42,10 @@ impl Default for ProxyType { } impl ProxyType { + /// Defines proxies permission hierarchy. + // Example: A proxy that is not superset of another one won't be able to remove + // that proxy relationship + // src: https://github.com/paritytech/polkadot-sdk/blob/4cd07c56378291fddb9fceab3b508cf99034126a/substrate/frame/proxy/src/lib.rs#L802 pub fn is_superset(s: &ProxyType, o: &ProxyType) -> bool { match (s, o) { (x, y) if x == y => true, From f3188404195d435c454be1c2c9ee1b157453e4b4 Mon Sep 17 00:00:00 2001 From: Alejandro Martinez Andres <11448715+al3mart@users.noreply.github.com> Date: Mon, 3 Feb 2025 14:10:53 +0100 Subject: [PATCH 9/9] test(proxy): clarify deposit byte size lenght --- runtime/mainnet/src/config/proxy.rs | 69 ++++++++++++++++------------- 1 file changed, 39 insertions(+), 30 deletions(-) diff --git a/runtime/mainnet/src/config/proxy.rs b/runtime/mainnet/src/config/proxy.rs index 1bb7c40e1..411d6f290 100644 --- a/runtime/mainnet/src/config/proxy.rs +++ b/runtime/mainnet/src/config/proxy.rs @@ -9,7 +9,7 @@ use crate::{ /// The type used to represent the kinds of proxying allowed. // Mainnet will use this definition of ProxyType instead of the ones in -// `pop-common` crates until `pallet-assets` is integrated in the runtime. +// `pop-common` crates until `pallet-assets` is in runtime. // `ProxyType` in `pop-common` include Assets specific proxies which won't // make much sense in this runtime. #[derive( @@ -87,8 +87,8 @@ parameter_types! { pub const ProxyDepositBase: Balance = deposit(1, 40); // Additional storage item size of AccountId 32 bytes + ProxyType 1 byte + BlockNum 4 bytes. pub const ProxyDepositFactor: Balance = deposit(0, 37); - // One storage item; key size 32, value size 16. - pub const AnnouncementDepositBase: Balance = deposit(1, 48); + // One storage item; key size 32, value size 16 + hash size 8. + pub const AnnouncementDepositBase: Balance = deposit(1, 56); // Additional storage item 32 bytes AccountId + 32 bytes Hash + 4 bytes BlockNum. pub const AnnouncementDepositFactor: Balance = deposit(0, 68); } @@ -112,9 +112,13 @@ impl pallet_proxy::Config for Runtime { mod tests { use std::any::TypeId; - use frame_support::traits::Get; + use frame_support::{traits::Get, StorageHasher, Twox64Concat}; + use pallet_proxy::Config; + use parachains_common::BlockNumber; + use sp_runtime::traits::Hash; use super::*; + use crate::AccountId; #[test] fn proxy_type_default_is_any() { @@ -149,74 +153,79 @@ mod tests { #[test] fn proxy_has_announcement_deposit_base() { + // AnnouncementDepositBase #bytes. + let base_bytes = Twox64Concat::max_len::() + Balance::max_encoded_len(); + assert_eq!(base_bytes, 56); + assert_eq!( - <::AnnouncementDepositBase as Get>::get(), - deposit(1, 48), + <::AnnouncementDepositBase as Get>::get(), + deposit(1, 56), ); } #[test] fn proxy_has_announcement_deposit_factor() { + // AnnouncementDepositFactor #bytes. + let factor_bytes = AccountId::max_encoded_len() + + <::CallHasher as Hash>::Output::max_encoded_len() + + BlockNumber::max_encoded_len(); + assert_eq!(factor_bytes, 68); + assert_eq!( - <::AnnouncementDepositFactor as Get>::get(), + <::AnnouncementDepositFactor as Get>::get(), deposit(0, 68), ); } #[test] fn proxy_uses_blaketwo256_as_hasher() { - assert_eq!( - TypeId::of::<::CallHasher>(), - TypeId::of::(), - ); + assert_eq!(TypeId::of::<::CallHasher>(), TypeId::of::(),); } #[test] fn proxy_uses_balances_as_currency() { - assert_eq!( - TypeId::of::<::Currency>(), - TypeId::of::(), - ); + assert_eq!(TypeId::of::<::Currency>(), TypeId::of::(),); } #[test] fn proxy_configures_max_pending() { - assert_eq!(<::MaxPending>::get(), 32,); + assert_eq!(<::MaxPending>::get(), 32,); } #[test] fn proxy_configures_max_num_of_proxies() { - assert_eq!(<::MaxProxies>::get(), 32,); + assert_eq!(<::MaxProxies>::get(), 32,); } #[test] fn proxy_has_deposit_base() { - assert_eq!( - <::ProxyDepositBase as Get>::get(), - deposit(1, 40), - ); + // ProxyDepositBase #bytes + let base_bytes = Twox64Concat::max_len::(); + assert_eq!(base_bytes, 40); + + assert_eq!(<::ProxyDepositBase as Get>::get(), deposit(1, 40),); } #[test] fn proxy_has_deposit_factor() { + // ProxyDepositFactor #bytes + let factor_bytes = AccountId::max_encoded_len() + + ProxyType::max_encoded_len() + + BlockNumber::max_encoded_len(); + assert_eq!(factor_bytes, 37); + assert_eq!( - <::ProxyDepositFactor as Get>::get(), + <::ProxyDepositFactor as Get>::get(), deposit(0, 37), ); } #[test] fn pallet_proxy_uses_proxy_type() { - assert_eq!( - TypeId::of::<::ProxyType>(), - TypeId::of::(), - ); + assert_eq!(TypeId::of::<::ProxyType>(), TypeId::of::(),); } #[test] fn proxy_does_not_use_default_weights() { - assert_ne!( - TypeId::of::<::WeightInfo>(), - TypeId::of::<()>(), - ); + assert_ne!(TypeId::of::<::WeightInfo>(), TypeId::of::<()>(),); } }