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 12 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
20 changes: 11 additions & 9 deletions .github/scripts/cmd/cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,20 +40,20 @@ def setup_logging():
setup_logging()

"""
BENCH
BENCH
"""

bench_example = '''**Examples**:
Runs all benchmarks
Runs all benchmarks
%(prog)s

Runs benchmarks for pallet_balances and pallet_multisig for all runtimes which have these pallets. **--quiet** makes it to output nothing to PR but reactions
%(prog)s --pallet pallet_balances pallet_xcm_benchmarks::generic --quiet

Runs bench for all pallets for westend runtime and fails fast on first failed benchmark
%(prog)s --runtime westend --fail-fast
Does not output anything and cleans up the previous bot's & author command triggering comments in PR

Does not output anything and cleans up the previous bot's & author command triggering comments in PR
%(prog)s --runtime westend rococo --pallet pallet_balances pallet_multisig --quiet --clean
'''

Expand All @@ -67,14 +67,14 @@ def setup_logging():
parser_bench.add_argument('--fail-fast', help='Fail fast on first failed benchmark', action='store_true')

"""
FMT
FMT
"""
parser_fmt = subparsers.add_parser('fmt', help='Formats code (cargo +nightly-VERSION fmt) and configs (taplo format)')
for arg, config in common_args.items():
parser_fmt.add_argument(arg, **config)

"""
Update UI
Update UI
"""
parser_ui = subparsers.add_parser('update-ui', help='Updates UI tests')
for arg, config in common_args.items():
Expand Down Expand Up @@ -175,7 +175,9 @@ def main():
print(f'-- package_dir: {package_dir}')
print(f'-- manifest_path: {manifest_path}')
output_path = os.path.join(package_dir, "src", "weights.rs")
template = config['template']
# TODO: we can remove once all pallets in dev runtime are migrated to polkadot-sdk-frame
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
uses_polkadot_sdk_frame = "true" in os.popen(f"cargo metadata --locked --format-version 1 --no-deps | jq -r '.packages[] | select(.name == \"{pallet.replace('_', '-')}\") | .dependencies | any(.name == \"polkadot-sdk-frame\")'").read()
template = config['template'] if not uses_polkadot_sdk_frame else "substrate/.maintain/frame-umbrella-weight-template.hbs"
else:
default_path = f"./{config['path']}/src/weights"
xcm_path = f"./{config['path']}/src/weights/xcm"
Expand Down Expand Up @@ -251,4 +253,4 @@ def main():
print('🚀 Done')

if __name__ == '__main__':
main()
main()
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.

4 changes: 2 additions & 2 deletions bridges/snowbridge/pallets/inbound-queue/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ use xcm_executor::traits::TransactAsset;
use snowbridge_core::{
inbound::{Message, VerificationError, Verifier},
sibling_sovereign_account, BasicOperatingMode, Channel, ChannelId, ParaId, PricingParameters,
StaticLookup,
StaticLookup as SnowBridgeStaticLookup,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This may have been due to my previous work.. Can you check if it can be reverted now?

Copy link
Contributor

Choose a reason for hiding this comment

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

reverted in 88265e5

};
use snowbridge_router_primitives::inbound::{
ConvertMessage, ConvertMessageError, VersionedMessage,
Expand Down Expand Up @@ -118,7 +118,7 @@ pub mod pallet {
>;

/// Lookup a channel descriptor
type ChannelLookup: StaticLookup<Source = ChannelId, Target = Channel>;
type ChannelLookup: SnowBridgeStaticLookup<Source = ChannelId, Target = Channel>;

/// Lookup pricing parameters
type PricingParameters: Get<PricingParameters<BalanceOf<Self>>>;
Expand Down
1 change: 0 additions & 1 deletion bridges/snowbridge/pallets/system/src/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
//! Governance API for controlling the Ethereum side of the bridge
use super::*;
use frame_support::traits::OnRuntimeUpgrade;
use log;

#[cfg(feature = "try-runtime")]
Expand Down
120 changes: 120 additions & 0 deletions substrate/.maintain/frame-umbrella-weight-template.hbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
{{header}}
//! Autogenerated weights for `{{pallet}}`
//!
//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION {{version}}
//! DATE: {{date}}, STEPS: `{{cmd.steps}}`, REPEAT: `{{cmd.repeat}}`, LOW RANGE: `{{cmd.lowest_range_values}}`, HIGH RANGE: `{{cmd.highest_range_values}}`
//! WORST CASE MAP SIZE: `{{cmd.worst_case_map_values}}`
//! HOSTNAME: `{{hostname}}`, CPU: `{{cpuname}}`
//! WASM-EXECUTION: `{{cmd.wasm_execution}}`, CHAIN: `{{cmd.chain}}`, DB CACHE: `{{cmd.db_cache}}`

// Executed Command:
{{#each args as |arg|}}
// {{arg}}
{{/each}}

#![cfg_attr(rustfmt, rustfmt_skip)]
#![allow(unused_parens)]
#![allow(unused_imports)]
#![allow(missing_docs)]

use frame::weights_prelude::*;

/// Weight functions needed for `{{pallet}}`.
pub trait WeightInfo {
{{#each benchmarks as |benchmark|}}
fn {{benchmark.name~}}
(
{{~#each benchmark.components as |c| ~}}
{{c.name}}: u32, {{/each~}}
) -> Weight;
{{/each}}
}

/// Weights for `{{pallet}}` using the Substrate node and recommended hardware.
pub struct SubstrateWeight<T>(PhantomData<T>);
{{#if (eq pallet "frame_system")}}
impl<T: crate::Config> WeightInfo for SubstrateWeight<T> {
{{else}}
impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
{{/if}}
{{#each benchmarks as |benchmark|}}
{{#each benchmark.comments as |comment|}}
/// {{comment}}
{{/each}}
{{#each benchmark.component_ranges as |range|}}
/// The range of component `{{range.name}}` is `[{{range.min}}, {{range.max}}]`.
{{/each}}
fn {{benchmark.name~}}
(
{{~#each benchmark.components as |c| ~}}
{{~#if (not c.is_used)}}_{{/if}}{{c.name}}: u32, {{/each~}}
) -> Weight {
// Proof Size summary in bytes:
// Measured: `{{benchmark.base_recorded_proof_size}}{{#each benchmark.component_recorded_proof_size as |cp|}} + {{cp.name}} * ({{cp.slope}} ±{{underscore cp.error}}){{/each}}`
// Estimated: `{{benchmark.base_calculated_proof_size}}{{#each benchmark.component_calculated_proof_size as |cp|}} + {{cp.name}} * ({{cp.slope}} ±{{underscore cp.error}}){{/each}}`
// Minimum execution time: {{underscore benchmark.min_execution_time}}_000 picoseconds.
Weight::from_parts({{underscore benchmark.base_weight}}, {{benchmark.base_calculated_proof_size}})
{{#each benchmark.component_weight as |cw|}}
// Standard Error: {{underscore cw.error}}
.saturating_add(Weight::from_parts({{underscore cw.slope}}, 0).saturating_mul({{cw.name}}.into()))
{{/each}}
{{#if (ne benchmark.base_reads "0")}}
.saturating_add(T::DbWeight::get().reads({{benchmark.base_reads}}_u64))
{{/if}}
{{#each benchmark.component_reads as |cr|}}
.saturating_add(T::DbWeight::get().reads(({{cr.slope}}_u64).saturating_mul({{cr.name}}.into())))
{{/each}}
{{#if (ne benchmark.base_writes "0")}}
.saturating_add(T::DbWeight::get().writes({{benchmark.base_writes}}_u64))
{{/if}}
{{#each benchmark.component_writes as |cw|}}
.saturating_add(T::DbWeight::get().writes(({{cw.slope}}_u64).saturating_mul({{cw.name}}.into())))
{{/each}}
{{#each benchmark.component_calculated_proof_size as |cp|}}
.saturating_add(Weight::from_parts(0, {{cp.slope}}).saturating_mul({{cp.name}}.into()))
{{/each}}
}
{{/each}}
}

// For backwards compatibility and tests.
impl WeightInfo for () {
{{#each benchmarks as |benchmark|}}
{{#each benchmark.comments as |comment|}}
/// {{comment}}
{{/each}}
{{#each benchmark.component_ranges as |range|}}
/// The range of component `{{range.name}}` is `[{{range.min}}, {{range.max}}]`.
{{/each}}
fn {{benchmark.name~}}
(
{{~#each benchmark.components as |c| ~}}
{{~#if (not c.is_used)}}_{{/if}}{{c.name}}: u32, {{/each~}}
) -> Weight {
// Proof Size summary in bytes:
// Measured: `{{benchmark.base_recorded_proof_size}}{{#each benchmark.component_recorded_proof_size as |cp|}} + {{cp.name}} * ({{cp.slope}} ±{{underscore cp.error}}){{/each}}`
// Estimated: `{{benchmark.base_calculated_proof_size}}{{#each benchmark.component_calculated_proof_size as |cp|}} + {{cp.name}} * ({{cp.slope}} ±{{underscore cp.error}}){{/each}}`
// Minimum execution time: {{underscore benchmark.min_execution_time}}_000 picoseconds.
Weight::from_parts({{underscore benchmark.base_weight}}, {{benchmark.base_calculated_proof_size}})
{{#each benchmark.component_weight as |cw|}}
// Standard Error: {{underscore cw.error}}
.saturating_add(Weight::from_parts({{underscore cw.slope}}, 0).saturating_mul({{cw.name}}.into()))
{{/each}}
{{#if (ne benchmark.base_reads "0")}}
.saturating_add(RocksDbWeight::get().reads({{benchmark.base_reads}}_u64))
{{/if}}
{{#each benchmark.component_reads as |cr|}}
.saturating_add(RocksDbWeight::get().reads(({{cr.slope}}_u64).saturating_mul({{cr.name}}.into())))
{{/each}}
{{#if (ne benchmark.base_writes "0")}}
.saturating_add(RocksDbWeight::get().writes({{benchmark.base_writes}}_u64))
{{/if}}
{{#each benchmark.component_writes as |cw|}}
.saturating_add(RocksDbWeight::get().writes(({{cw.slope}}_u64).saturating_mul({{cw.name}}.into())))
{{/each}}
{{#each benchmark.component_calculated_proof_size as |cp|}}
.saturating_add(Weight::from_parts(0, {{cp.slope}}).saturating_mul({{cp.name}}.into()))
{{/each}}
}
{{/each}}
}
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",
"sp-inherents?/std",
"sp-io/std",
"sp-offchain?/std",
Expand Down
8 changes: 7 additions & 1 deletion substrate/frame/democracy/src/migrations/v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,13 @@
//! Storage migrations for the preimage pallet.

use crate::*;
use frame_support::{pallet_prelude::*, storage_alias, traits::OnRuntimeUpgrade, BoundedVec};
use frame_support::{
// We cannot use the `pallet_prelude` here as the item `Bounded` will become ambiguous.
pallet_prelude::{StorageVersion, ValueQuery},
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if i follow. You mean pallet_prelude::* errors? I tried it locally and seems fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

True, because we removed Bounded from frame_support::pallet_prelude::*. Reverted in d460295

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a good example of where adding more things to frame_support::pallet_prelude::* turned out to be a headache.

storage_alias,
traits::OnRuntimeUpgrade,
BoundedVec,
};
use frame_system::pallet_prelude::BlockNumberFor;
use sp_core::H256;

Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/migrations/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ mod benches {
fn on_init_loop() {
T::Migrations::set_fail_after(0); // Should not be called anyway.
System::<T>::set_block_number(1u32.into());
Pallet::<T>::on_runtime_upgrade();
<Pallet<T> as Hooks<BlockNumberFor<T>>>::on_runtime_upgrade();

#[block]
{
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",
]
Loading
Loading