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

refactor(mainnet): pallet_proxy config & tests #441

Merged
103 changes: 97 additions & 6 deletions runtime/mainnet/src/config/proxy.rs
Original file line number Diff line number Diff line change
@@ -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<RuntimeCall> for ProxyType {
fn filter(&self, c: &RuntimeCall) -> bool {
Expand Down Expand Up @@ -41,6 +39,17 @@ impl InstanceFilter<RuntimeCall> for ProxyType {
}
}

parameter_types! {
// One storage item; key size 32, value size 16.
pub const ProxyDepositBase: Balance = deposit(1, 48);
Copy link
Collaborator

Choose a reason for hiding this comment

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

we have 6 times the amount of bytes Polkadot has. Any reason not to just align to the Polkadot relay chain on this: https://github.com/polkadot-fellows/runtimes/blob/main/relay/polkadot/src/lib.rs#L977

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Certainly. Double checking after your comment I'm also seeing that I did the following miscalculation:

the number we have for ProxyDepositBase was the result of adding AccountId + Balance, but I realize that we should be accounting for 8 bytes, which is the correct length of the key of the map.

I also added the bytes for Balance because the deposited amount is always stored. Although, as it is the size corresponding to the deposit itself, I could see why it is not being included on Polkadot's configuration.

ProxyDepositFactor reasoning in the comment above its declaration, what's the concern ?

Copy link
Collaborator Author

@al3mart al3mart Jan 29, 2025

Choose a reason for hiding this comment

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

d4c936e

As discussed off GH this change updates ProxyDepositBase from deposit(1, 48) to 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);
// 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;
Expand All @@ -55,3 +64,85 @@ impl pallet_proxy::Config for Runtime {
type RuntimeEvent = RuntimeEvent;
type WeightInfo = pallet_proxy::weights::SubstrateWeight<Self>;
}

#[cfg(test)]
mod tests {
use std::any::TypeId;

use frame_support::traits::Get;

use super::*;

#[test]
fn proxy_has_announcement_deposit_base() {
assert_eq!(
<<Runtime as pallet_proxy::Config>::AnnouncementDepositBase as Get<Balance>>::get(),
deposit(1, 48),
);
}
#[test]
fn proxy_has_announcement_deposit_factor() {
assert_eq!(
<<Runtime as pallet_proxy::Config>::AnnouncementDepositFactor as Get<Balance>>::get(),
deposit(0, 68),
);
}

#[test]
fn proxy_uses_blaketwo256_as_hasher() {
assert_eq!(
TypeId::of::<<Runtime as pallet_proxy::Config>::CallHasher>(),
TypeId::of::<BlakeTwo256>(),
);
}

#[test]
fn proxy_uses_balances_as_currency() {
assert_eq!(
TypeId::of::<<Runtime as pallet_proxy::Config>::Currency>(),
TypeId::of::<Balances>(),
);
}

#[test]
fn proxy_configures_max_pending() {
assert_eq!(<<Runtime as pallet_proxy::Config>::MaxPending>::get(), 32,);
}

#[test]
fn proxy_configures_max_num_of_proxies() {
assert_eq!(<<Runtime as pallet_proxy::Config>::MaxProxies>::get(), 32,);
}

#[test]
fn proxy_has_deposit_base() {
assert_eq!(
<<Runtime as pallet_proxy::Config>::ProxyDepositBase as Get<Balance>>::get(),
deposit(1, 48),
);
}

#[test]
fn proxy_has_deposit_factor() {
assert_eq!(
<<Runtime as pallet_proxy::Config>::ProxyDepositFactor as Get<Balance>>::get(),
deposit(0, 37),
);
}

#[test]
fn pallet_proxy_uses_proxy_type() {
assert_eq!(
TypeId::of::<<Runtime as pallet_proxy::Config>::ProxyType>(),
TypeId::of::<ProxyType>(),
);
}

#[test]
fn proxy_does_not_use_default_weights() {
assert_ne!(
TypeId::of::<<Runtime as pallet_proxy::Config>::WeightInfo>(),
TypeId::of::<()>(),
);
}
}
Loading