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

Use frame umbrella crate in pallet-proxy and pallet-multisig #5995

Merged
merged 44 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
27df8ea
pallet-proxy
kianenigma Oct 9, 2024
8eefc00
pallet-multisig
kianenigma Oct 9, 2024
c151681
Merge branch 'master' of github.com:paritytech/polkadot-sdk into kiz-…
kianenigma Oct 9, 2024
78b52f2
better docs
kianenigma Oct 9, 2024
0305c59
Merge branch 'master' of github.com:paritytech/polkadot-sdk into kiz-…
kianenigma Oct 9, 2024
d253bc5
also improve the minimal template
kianenigma Oct 9, 2024
a4b1fd2
fix
kianenigma Oct 9, 2024
e0293d1
Update substrate/frame/multisig/src/migrations.rs
kianenigma Oct 15, 2024
253ce3f
Upgrade Proxy and Multisig pallets to Benchmarking V2 (#6018)
re-gius Oct 15, 2024
ff36c53
cleanup
kianenigma Oct 15, 2024
2d8323f
possibly how CI will look like
kianenigma Oct 15, 2024
687b61f
Master.into()
kianenigma Oct 16, 2024
49a4cc3
run_all_benchmarks.sh: format + use umbrella template when possible
re-gius Oct 21, 2024
4debb8e
Merge branch 'master' into kiz-frame-umbrella-in-pallets
re-gius Oct 21, 2024
556e8ea
fix Issue #6160
re-gius Oct 21, 2024
a6dfbed
remove unused code in multisig/benchmarking
re-gius Oct 21, 2024
645f2f4
fix `cmd.py` to use umbrella weights template only in `frame`
re-gius Oct 22, 2024
21cd6da
fix CI: handle empty output from `os.popen`
re-gius Oct 22, 2024
2c910cb
moving imports from support to umbrella crate
re-gius Oct 22, 2024
6e6f1ac
Revert "moving imports from support to umbrella crate"
re-gius Oct 22, 2024
bd17cf2
undo frame support prelude edits + fix imports
re-gius Oct 23, 2024
532ee9b
fix and fmt `Cargo.toml` files + remove duplicate import
re-gius Oct 23, 2024
d0120f2
Add prdoc
re-gius Oct 23, 2024
1e7d1bb
docs fix
re-gius Oct 23, 2024
4d8b583
Merge branch 'master' into kiz-frame-umbrella-in-pallets
re-gius Oct 23, 2024
cf32fbf
undo docs change
re-gius Oct 23, 2024
d460295
add docs comment + undo format for `run_all_benchmarks.sh`
re-gius Oct 24, 2024
ca35eed
Update substrate/frame/src/lib.rs
re-gius Oct 24, 2024
68c5eea
Merge branch 'master' into kiz-frame-umbrella-in-pallets
re-gius Oct 24, 2024
393d998
comment nit
re-gius Oct 24, 2024
2f5eea9
Update prdoc/pr_5995.prdoc
re-gius Oct 24, 2024
b85d644
Update substrate/frame/src/lib.rs
re-gius Oct 24, 2024
9e771ee
Update prdoc/pr_5995.prdoc
re-gius Oct 24, 2024
66d7396
Update prdoc/pr_5995.prdoc
re-gius Oct 24, 2024
88265e5
undo snowbridge changes
re-gius Oct 24, 2024
dc2973d
expand frame docs
re-gius Oct 24, 2024
8cd8c73
remove broken links from docs
re-gius Oct 24, 2024
5918bef
fix docs + add storage value to minimal template
re-gius Oct 24, 2024
325d62e
prdoc add missing pallet
re-gius Oct 24, 2024
801156c
Merge branch 'master' into kiz-frame-umbrella-in-pallets
re-gius Oct 24, 2024
48050fd
docs nit
re-gius Oct 25, 2024
d9f132b
Apply suggestions from code review
kianenigma Oct 25, 2024
61db970
Merge branch 'master' into kiz-frame-umbrella-in-pallets
kianenigma Oct 28, 2024
f5ba680
Merge branch 'master' into kiz-frame-umbrella-in-pallets
re-gius Oct 29, 2024
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
14 changes: 3 additions & 11 deletions Cargo.lock

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

3 changes: 3 additions & 0 deletions substrate/frame/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ sp-consensus-aura = { optional = true, workspace = true }
sp-consensus-grandpa = { optional = true, workspace = true }
sp-inherents = { optional = true, workspace = true }
sp-storage = { optional = true, workspace = true }
sp-genesis-builder = { optional = true, workspace = true }

frame-executive = { optional = true, workspace = true }
frame-system-rpc-runtime-api = { optional = true, workspace = true }
Expand Down Expand Up @@ -77,6 +78,7 @@ runtime = [
"sp-storage",
"sp-transaction-pool",
"sp-version",
"sp-genesis-builder",

"frame-executive",
"frame-system-rpc-runtime-api",
Expand All @@ -98,6 +100,7 @@ std = [
"sp-consensus-aura?/std",
"sp-consensus-grandpa?/std",
"sp-core/std",
"sp-genesis-builder/std",
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
"sp-inherents?/std",
"sp-io/std",
"sp-offchain?/std",
Expand Down
25 changes: 4 additions & 21 deletions substrate/frame/multisig/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,7 @@ targets = ["x86_64-unknown-linux-gnu"]
[dependencies]
codec = { workspace = true }
scale-info = { features = ["derive"], workspace = true }
frame-benchmarking = { optional = true, workspace = true }
frame-support = { workspace = true }
frame-system = { workspace = true }
sp-io = { workspace = true }
sp-runtime = { workspace = true }
frame = { workspace = true, features = ["experimental", "runtime"] }
Copy link
Contributor

Choose a reason for hiding this comment

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

do we actually need experimental? I don't see it used in the past AFAICT

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will remove it as soon as I am a bit more confident in the readiness of the frame crate 🙈

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What we should try here is to see if removing the feature = runtime still works?

The idea in the frame umbrella crate was that for pallet development, you would only need crate itself, and for a runtime, the crate plus runtime feature.


# third party
log = { workspace = true }
Expand All @@ -34,25 +30,12 @@ pallet-balances = { workspace = true, default-features = true }
default = ["std"]
std = [
"codec/std",
"frame-benchmarking?/std",
"frame-support/std",
"frame-system/std",
"log/std",
"pallet-balances/std",
"scale-info/std",
"sp-io/std",
"sp-runtime/std",
"frame/std",
]
runtime-benchmarks = [
"frame-benchmarking/runtime-benchmarks",
"frame-support/runtime-benchmarks",
"frame-system/runtime-benchmarks",
"pallet-balances/runtime-benchmarks",
"sp-runtime/runtime-benchmarks",
"frame/runtime-benchmarks",
]
try-runtime = [
"frame-support/try-runtime",
"frame-system/try-runtime",
"pallet-balances/try-runtime",
"sp-runtime/try-runtime",
"frame/try-runtime",
]
19 changes: 9 additions & 10 deletions substrate/frame/multisig/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@
#![cfg(feature = "runtime-benchmarks")]

use super::*;
use frame_benchmarking::v1::{account, benchmarks};
use frame_system::RawOrigin;
use sp_runtime::traits::Bounded;
#[allow(deprecated)]
use frame::benchmarking::v1::*;

use crate::Pallet as Multisig;

Expand Down Expand Up @@ -61,7 +60,7 @@ benchmarks! {
let caller = signatories.pop().ok_or("signatories should have len 2 or more")?;
// Whitelist caller account from further DB operations.
let caller_key = frame_system::Account::<T>::hashed_key_for(&caller);
frame_benchmarking::benchmarking::add_to_whitelist(caller_key.into());
add_to_whitelist(caller_key.into());
}: _(RawOrigin::Signed(caller.clone()), signatories, Box::new(call))
verify {
// If the benchmark resolves, then the call was dispatched successfully.
Expand All @@ -78,7 +77,7 @@ benchmarks! {
let caller = signatories.pop().ok_or("signatories should have len 2 or more")?;
// Whitelist caller account from further DB operations.
let caller_key = frame_system::Account::<T>::hashed_key_for(&caller);
frame_benchmarking::benchmarking::add_to_whitelist(caller_key.into());
add_to_whitelist(caller_key.into());
}: as_multi(RawOrigin::Signed(caller), s as u16, signatories, None, call, Weight::zero())
verify {
assert!(Multisigs::<T>::contains_key(multi_account_id, call_hash));
Expand All @@ -101,7 +100,7 @@ benchmarks! {
let caller2 = signatories2.remove(0);
// Whitelist caller account from further DB operations.
let caller_key = frame_system::Account::<T>::hashed_key_for(&caller2);
frame_benchmarking::benchmarking::add_to_whitelist(caller_key.into());
add_to_whitelist(caller_key.into());
}: as_multi(RawOrigin::Signed(caller2), s as u16, signatories2, Some(timepoint), call, Weight::zero())
verify {
let multisig = Multisigs::<T>::get(multi_account_id, call_hash).ok_or("multisig not created")?;
Expand Down Expand Up @@ -133,7 +132,7 @@ benchmarks! {
assert!(Multisigs::<T>::contains_key(&multi_account_id, call_hash));
// Whitelist caller account from further DB operations.
let caller_key = frame_system::Account::<T>::hashed_key_for(&caller2);
frame_benchmarking::benchmarking::add_to_whitelist(caller_key.into());
add_to_whitelist(caller_key.into());
}: as_multi(RawOrigin::Signed(caller2), s as u16, signatories2, Some(timepoint), call, Weight::MAX)
verify {
assert!(!Multisigs::<T>::contains_key(&multi_account_id, call_hash));
Expand All @@ -150,7 +149,7 @@ benchmarks! {
let call_hash = call.using_encoded(blake2_256);
// Whitelist caller account from further DB operations.
let caller_key = frame_system::Account::<T>::hashed_key_for(&caller);
frame_benchmarking::benchmarking::add_to_whitelist(caller_key.into());
add_to_whitelist(caller_key.into());
// Create the multi
}: approve_as_multi(RawOrigin::Signed(caller), s as u16, signatories, None, call_hash, Weight::zero())
verify {
Expand Down Expand Up @@ -181,7 +180,7 @@ benchmarks! {
let caller2 = signatories2.remove(0);
// Whitelist caller account from further DB operations.
let caller_key = frame_system::Account::<T>::hashed_key_for(&caller2);
frame_benchmarking::benchmarking::add_to_whitelist(caller_key.into());
add_to_whitelist(caller_key.into());
}: approve_as_multi(RawOrigin::Signed(caller2), s as u16, signatories2, Some(timepoint), call_hash, Weight::zero())
verify {
let multisig = Multisigs::<T>::get(multi_account_id, call_hash).ok_or("multisig not created")?;
Expand All @@ -204,7 +203,7 @@ benchmarks! {
assert!(Multisigs::<T>::contains_key(&multi_account_id, call_hash));
// Whitelist caller account from further DB operations.
let caller_key = frame_system::Account::<T>::hashed_key_for(&caller);
frame_benchmarking::benchmarking::add_to_whitelist(caller_key.into());
add_to_whitelist(caller_key.into());
}: _(RawOrigin::Signed(caller), s as u16, signatories, timepoint, call_hash)
verify {
assert!(!Multisigs::<T>::contains_key(multi_account_id, call_hash));
Expand Down
34 changes: 9 additions & 25 deletions substrate/frame/multisig/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,28 +49,14 @@ mod tests;
pub mod weights;

extern crate alloc;

use alloc::{boxed::Box, vec, vec::Vec};
use codec::{Decode, Encode, MaxEncodedLen};
use frame_support::{
dispatch::{
DispatchErrorWithPostInfo, DispatchResult, DispatchResultWithPostInfo, GetDispatchInfo,
PostDispatchInfo,
},
ensure,
traits::{Currency, Get, ReservableCurrency},
weights::Weight,
BoundedVec,
};
use frame_system::{self as system, pallet_prelude::BlockNumberFor, RawOrigin};
use scale_info::TypeInfo;
use sp_io::hashing::blake2_256;
use sp_runtime::{
traits::{Dispatchable, TrailingZeroInput, Zero},
DispatchError, RuntimeDebug,
use frame::{
prelude::*,
traits::{Currency, ReservableCurrency},
};
pub use weights::WeightInfo;
use weights::WeightInfo;
kianenigma marked this conversation as resolved.
Show resolved Hide resolved

/// Re-export all pallet items.
pub use pallet::*;

/// The log target of this pallet.
Expand Down Expand Up @@ -127,11 +113,9 @@ enum CallOrHash<T: Config> {
Hash([u8; 32]),
}

#[frame_support::pallet]
#[frame::pallet]
pub mod pallet {
use super::*;
use frame_support::pallet_prelude::*;
use frame_system::pallet_prelude::*;

#[pallet::config]
pub trait Config: frame_system::Config {
Expand Down Expand Up @@ -167,7 +151,7 @@ pub mod pallet {
type MaxSignatories: Get<u32>;

/// Weight information for extrinsics in this pallet.
type WeightInfo: WeightInfo;
type WeightInfo: weights::WeightInfo;
}

/// The in-code storage version.
Expand Down Expand Up @@ -641,8 +625,8 @@ impl<T: Config> Pallet<T> {
/// The current `Timepoint`.
pub fn timepoint() -> Timepoint<BlockNumberFor<T>> {
Timepoint {
height: <system::Pallet<T>>::block_number(),
index: <system::Pallet<T>>::extrinsic_index().unwrap_or_default(),
height: <frame_system::Pallet<T>>::block_number(),
index: <frame_system::Pallet<T>>::extrinsic_index().unwrap_or_default(),
}
}

Expand Down
21 changes: 7 additions & 14 deletions substrate/frame/multisig/src/migrations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,15 @@

// Migrations for Multisig Pallet

use super::*;
use frame_support::{
traits::{GetStorageVersion, OnRuntimeUpgrade, WrapperKeepOpaque},
Identity,
};

#[cfg(feature = "try-runtime")]
use frame_support::ensure;
use crate::*;
use frame::prelude::*;

pub mod v1 {
use super::*;

type OpaqueCall<T> = WrapperKeepOpaque<<T as Config>::RuntimeCall>;
type OpaqueCall<T> = frame::traits::WrapperKeepOpaque<<T as Config>::RuntimeCall>;

#[frame_support::storage_alias]
#[frame::storage_alias]
type Calls<T: Config> = StorageMap<
Pallet<T>,
Identity,
Expand All @@ -42,15 +36,14 @@ pub mod v1 {
pub struct MigrateToV1<T>(core::marker::PhantomData<T>);
impl<T: Config> OnRuntimeUpgrade for MigrateToV1<T> {
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, sp_runtime::TryRuntimeError> {
fn pre_upgrade() -> Result<Vec<u8>, frame::deps::sp_runtime::TryRuntimeError> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not related to the PR, but maybe we could introduce: TryRuntimeResult<T> = Result<T, TryRuntimeError>

kianenigma marked this conversation as resolved.
Show resolved Hide resolved
log!(info, "Number of calls to refund and delete: {}", Calls::<T>::iter().count());

Ok(Vec::new())
}

fn on_runtime_upgrade() -> Weight {
use sp_runtime::Saturating;

use frame::traits::ReservableCurrency as _;
let current = Pallet::<T>::in_code_storage_version();
let onchain = Pallet::<T>::on_chain_storage_version();

Expand All @@ -76,7 +69,7 @@ pub mod v1 {
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(_state: Vec<u8>) -> Result<(), sp_runtime::TryRuntimeError> {
fn post_upgrade(_state: Vec<u8>) -> Result<(), frame::try_runtime::TryRuntimeError> {
ensure!(
Calls::<T>::iter().count() == 0,
"there are some dangling calls that need to be destroyed and refunded"
Expand Down
15 changes: 5 additions & 10 deletions substrate/frame/multisig/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,13 @@
#![cfg(test)]

use super::*;

use crate as pallet_multisig;
use frame_support::{
assert_noop, assert_ok, derive_impl,
traits::{ConstU32, ConstU64, Contains},
};
use sp_runtime::{BuildStorage, TokenError};
use frame::{prelude::*, runtime::prelude::*, testing_prelude::*};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

One little detail here is which preludes are a superset of the others.

I recall I made a deliberate choice about preludes being a superset of one another, but I didn't update the code 🙈 For example here, I think testing_prelude brings in the other two


type Block = frame_system::mocking::MockBlockU32<Test>;

frame_support::construct_runtime!(
pub enum Test {
construct_runtime!(
pub struct Test {
System: frame_system,
Balances: pallet_balances,
Multisig: pallet_multisig,
Expand Down Expand Up @@ -75,14 +70,14 @@ impl Config for Test {

use pallet_balances::Call as BalancesCall;

pub fn new_test_ext() -> sp_io::TestExternalities {
pub fn new_test_ext() -> TestState {
let mut t = frame_system::GenesisConfig::<Test>::default().build_storage().unwrap();
pallet_balances::GenesisConfig::<Test> {
balances: vec![(1, 10), (2, 10), (3, 10), (4, 10), (5, 2)],
}
.assimilate_storage(&mut t)
.unwrap();
let mut ext = sp_io::TestExternalities::new(t);
let mut ext = TestState::new(t);
ext.execute_with(|| System::set_block_number(1));
ext
}
Expand Down
5 changes: 2 additions & 3 deletions substrate/frame/multisig/src/weights.rs

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

Loading
Loading