Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Whitelist pallet preimage provider upgrade #12834

Merged
merged 6 commits into from
Dec 6, 2022
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 1 addition & 1 deletion bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1544,7 +1544,7 @@ impl pallet_whitelist::Config for Runtime {
type RuntimeCall = RuntimeCall;
type WhitelistOrigin = EnsureRoot<AccountId>;
type DispatchWhitelistedOrigin = EnsureRoot<AccountId>;
type PreimageProvider = Preimage;
type Preimages = Preimage;
type WeightInfo = pallet_whitelist::weights::SubstrateWeight<Runtime>;
}

Expand Down
29 changes: 13 additions & 16 deletions frame/whitelist/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,7 @@

use super::*;
use frame_benchmarking::benchmarks;
use frame_support::{
ensure,
traits::{EnsureOrigin, Get, PreimageRecipient},
};
use frame_support::{ensure, traits::EnsureOrigin};

#[cfg(test)]
use crate::Pallet as Whitelist;
Expand All @@ -40,7 +37,7 @@ benchmarks! {
"call not whitelisted"
);
ensure!(
T::PreimageProvider::preimage_requested(&call_hash),
<T::Preimages as QueryPreimage>::is_requested(&call_hash),
Copy link
Member

Choose a reason for hiding this comment

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

nit

Suggested change
<T::Preimages as QueryPreimage>::is_requested(&call_hash),
T::Preimages::is_requested(&call_hash),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated it everywhere.

"preimage not requested"
);
}
Expand All @@ -57,7 +54,7 @@ benchmarks! {
"whitelist not removed"
);
ensure!(
!T::PreimageProvider::preimage_requested(&call_hash),
!<T::Preimages as QueryPreimage>::is_requested(&call_hash),
"preimage still requested"
);
}
Expand All @@ -66,30 +63,30 @@ benchmarks! {
// If the resulting weight is too big, maybe it worth having a weight which depends
// on the size of the call, with a new witness in parameter.
dispatch_whitelisted_call {
let origin = T::DispatchWhitelistedOrigin::successful_origin();
// NOTE: we remove `10` because we need some bytes to encode the variants and vec length
let remark_len = <T::PreimageProvider as PreimageRecipient<_>>::MaxSize::get() - 10;
let remark = sp_std::vec![1u8; remark_len as usize];
let n in 1 .. <T::Preimages as StorePreimage>::MAX_LENGTH as u32 - 10;

let origin = T::DispatchWhitelistedOrigin::successful_origin();
let remark = sp_std::vec![1u8; n as usize];
let call: <T as Config>::RuntimeCall = frame_system::Call::remark { remark }.into();
let call_weight = call.get_dispatch_info().weight;
let encoded_call = call.encode();
let call_hash = T::Hashing::hash(&encoded_call[..]);
let call_encoded_len = encoded_call.len() as u32;
let call_hash = call.blake2_256().into();

Pallet::<T>::whitelist_call(origin.clone(), call_hash)
.expect("whitelisting call must be successful");

let encoded_call = encoded_call.try_into().expect("encoded_call must be small enough");
T::PreimageProvider::note_preimage(encoded_call);
<T::Preimages as StorePreimage>::note(encoded_call.into()).unwrap();

}: _<T::RuntimeOrigin>(origin, call_hash, call_weight)
}: _<T::RuntimeOrigin>(origin, call_hash, call_encoded_len, call_weight)
verify {
ensure!(
!WhitelistedCall::<T>::contains_key(call_hash),
"whitelist not removed"
);
ensure!(
!T::PreimageProvider::preimage_requested(&call_hash),
!<T::Preimages as QueryPreimage>::is_requested(&call_hash),
"preimage still requested"
);
}
Expand All @@ -101,7 +98,7 @@ benchmarks! {
let remark = sp_std::vec![1u8; n as usize];

let call: <T as Config>::RuntimeCall = frame_system::Call::remark { remark }.into();
let call_hash = T::Hashing::hash_of(&call);
let call_hash = call.blake2_256().into();

Pallet::<T>::whitelist_call(origin.clone(), call_hash)
.expect("whitelisting call must be successful");
Expand All @@ -112,7 +109,7 @@ benchmarks! {
"whitelist not removed"
);
ensure!(
!T::PreimageProvider::preimage_requested(&call_hash),
!<T::Preimages as QueryPreimage>::is_requested(&call_hash),
"preimage still requested"
);
}
Expand Down
47 changes: 25 additions & 22 deletions frame/whitelist/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
//! with the root origin.
//!
//! In the meantime the call corresponding to the hash must have been submitted to the pre-image
//! handler [`PreimageProvider`].
//! handler [`pallet::Config::Preimages`].

#![cfg_attr(not(feature = "std"), no_std)]

Expand All @@ -44,11 +44,12 @@ use codec::{DecodeLimit, Encode, FullCodec};
use frame_support::{
dispatch::{GetDispatchInfo, PostDispatchInfo},
ensure,
traits::{PreimageProvider, PreimageRecipient},
traits::{Hash, QueryPreimage, StorePreimage},
weights::Weight,
Hashable,
};
use scale_info::TypeInfo;
use sp_runtime::traits::{Dispatchable, Hash};
use sp_runtime::traits::Dispatchable;
use sp_std::prelude::*;

pub use pallet::*;
Expand Down Expand Up @@ -80,8 +81,7 @@ pub mod pallet {
type DispatchWhitelistedOrigin: EnsureOrigin<Self::RuntimeOrigin>;

/// The handler of pre-images.
// NOTE: recipient is only needed for benchmarks.
type PreimageProvider: PreimageProvider<Self::Hash> + PreimageRecipient<Self::Hash>;
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
type Preimages: QueryPreimage + StorePreimage;

/// The weight information for this pallet.
type WeightInfo: WeightInfo;
Expand All @@ -94,9 +94,9 @@ pub mod pallet {
#[pallet::event]
#[pallet::generate_deposit(pub(super) fn deposit_event)]
pub enum Event<T: Config> {
CallWhitelisted { call_hash: T::Hash },
WhitelistedCallRemoved { call_hash: T::Hash },
WhitelistedCallDispatched { call_hash: T::Hash, result: DispatchResultWithPostInfo },
CallWhitelisted { call_hash: Hash },
WhitelistedCallRemoved { call_hash: Hash },
WhitelistedCallDispatched { call_hash: Hash, result: DispatchResultWithPostInfo },
Copy link
Member

Choose a reason for hiding this comment

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

Maybe you can change these to H256 to easier spot that it is not the standard T::Hash.

Copy link
Member

Choose a reason for hiding this comment

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

Could also be preimages::Hash as PreimageHash

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

}

#[pallet::error]
Expand All @@ -114,12 +114,12 @@ pub mod pallet {
}

#[pallet::storage]
pub type WhitelistedCall<T: Config> = StorageMap<_, Twox64Concat, T::Hash, (), OptionQuery>;
pub type WhitelistedCall<T: Config> = StorageMap<_, Twox64Concat, Hash, (), OptionQuery>;
Copy link
Member

@ggwpez ggwpez Dec 5, 2022

Choose a reason for hiding this comment

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

I think we always use Hash = H256 but it would need a migration if some chains use something different.
Probably noteworthy for the change-log.

Copy link
Member

Choose a reason for hiding this comment

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

I very much doubt any are.


#[pallet::call]
impl<T: Config> Pallet<T> {
#[pallet::weight(T::WeightInfo::whitelist_call())]
pub fn whitelist_call(origin: OriginFor<T>, call_hash: T::Hash) -> DispatchResult {
pub fn whitelist_call(origin: OriginFor<T>, call_hash: Hash) -> DispatchResult {
T::WhitelistOrigin::ensure_origin(origin)?;

ensure!(
Expand All @@ -128,32 +128,34 @@ pub mod pallet {
);

WhitelistedCall::<T>::insert(call_hash, ());
T::PreimageProvider::request_preimage(&call_hash);
T::Preimages::request(&call_hash);

Self::deposit_event(Event::<T>::CallWhitelisted { call_hash });

Ok(())
}

#[pallet::weight(T::WeightInfo::remove_whitelisted_call())]
pub fn remove_whitelisted_call(origin: OriginFor<T>, call_hash: T::Hash) -> DispatchResult {
pub fn remove_whitelisted_call(origin: OriginFor<T>, call_hash: Hash) -> DispatchResult {
T::WhitelistOrigin::ensure_origin(origin)?;

WhitelistedCall::<T>::take(call_hash).ok_or(Error::<T>::CallIsNotWhitelisted)?;

T::PreimageProvider::unrequest_preimage(&call_hash);
T::Preimages::unrequest(&call_hash);

Self::deposit_event(Event::<T>::WhitelistedCallRemoved { call_hash });

Ok(())
}

#[pallet::weight(
T::WeightInfo::dispatch_whitelisted_call().saturating_add(*call_weight_witness)
T::WeightInfo::dispatch_whitelisted_call(*call_encoded_len)
.saturating_add(*call_weight_witness)
)]
pub fn dispatch_whitelisted_call(
origin: OriginFor<T>,
call_hash: T::Hash,
call_hash: Hash,
call_encoded_len: u32,
call_weight_witness: Weight,
) -> DispatchResultWithPostInfo {
T::DispatchWhitelistedOrigin::ensure_origin(origin)?;
Expand All @@ -163,8 +165,8 @@ pub mod pallet {
Error::<T>::CallIsNotWhitelisted,
);

let call = T::PreimageProvider::get_preimage(&call_hash)
.ok_or(Error::<T>::UnavailablePreImage)?;
let call = T::Preimages::fetch(&call_hash, Some(call_encoded_len))
.map_err(|_| Error::<T>::UnavailablePreImage)?;

let call = <T as Config>::RuntimeCall::decode_all_with_depth_limit(
sp_api::MAX_EXTRINSIC_DEPTH,
Expand All @@ -177,8 +179,9 @@ pub mod pallet {
Error::<T>::InvalidCallWeightWitness
);

let actual_weight = Self::clean_and_dispatch(call_hash, call)
.map(|w| w.saturating_add(T::WeightInfo::dispatch_whitelisted_call()));
let actual_weight = Self::clean_and_dispatch(call_hash, call).map(|w| {
w.saturating_add(T::WeightInfo::dispatch_whitelisted_call(call_encoded_len))
});

Ok(actual_weight.into())
}
Expand All @@ -196,7 +199,7 @@ pub mod pallet {
) -> DispatchResultWithPostInfo {
T::DispatchWhitelistedOrigin::ensure_origin(origin)?;

let call_hash = <T as frame_system::Config>::Hashing::hash_of(&call);
let call_hash = call.blake2_256().into();

ensure!(
WhitelistedCall::<T>::contains_key(call_hash),
Expand All @@ -217,10 +220,10 @@ impl<T: Config> Pallet<T> {
/// Clean whitelisting/preimage and dispatch call.
///
/// Return the call actual weight of the dispatched call if there is some.
fn clean_and_dispatch(call_hash: T::Hash, call: <T as Config>::RuntimeCall) -> Option<Weight> {
fn clean_and_dispatch(call_hash: Hash, call: <T as Config>::RuntimeCall) -> Option<Weight> {
WhitelistedCall::<T>::remove(call_hash);

T::PreimageProvider::unrequest_preimage(&call_hash);
T::Preimages::unrequest(&call_hash);

let result = call.dispatch(frame_system::Origin::<T>::Root.into());

Expand Down
2 changes: 1 addition & 1 deletion frame/whitelist/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ impl pallet_whitelist::Config for Test {
type RuntimeCall = RuntimeCall;
type WhitelistOrigin = EnsureRoot<Self::AccountId>;
type DispatchWhitelistedOrigin = EnsureRoot<Self::AccountId>;
type PreimageProvider = Preimage;
type Preimages = Preimage;
type WeightInfo = ();
}

Expand Down
Loading