diff --git a/Cargo.lock b/Cargo.lock index b46cf17e6a4a4..740e45a0d5333 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2804,6 +2804,7 @@ dependencies = [ name = "frame-system" version = "4.0.0-dev" dependencies = [ + "cfg-if", "criterion", "frame-support", "log", @@ -2830,9 +2831,11 @@ dependencies = [ "parity-scale-codec", "scale-info", "sp-core", + "sp-externalities", "sp-io", "sp-runtime", "sp-std", + "sp-version", ] [[package]] diff --git a/frame/system/Cargo.toml b/frame/system/Cargo.toml index c97eb66382ed6..1fa52de10e85c 100644 --- a/frame/system/Cargo.toml +++ b/frame/system/Cargo.toml @@ -13,6 +13,7 @@ readme = "README.md" targets = ["x86_64-unknown-linux-gnu"] [dependencies] +cfg-if = "1.0" codec = { package = "parity-scale-codec", version = "3.2.2", default-features = false, features = ["derive"] } log = { version = "0.4.17", default-features = false } scale-info = { version = "2.5.0", default-features = false, features = ["derive"] } diff --git a/frame/system/benchmarking/Cargo.toml b/frame/system/benchmarking/Cargo.toml index 50009387a2088..8b189848d8f91 100644 --- a/frame/system/benchmarking/Cargo.toml +++ b/frame/system/benchmarking/Cargo.toml @@ -24,6 +24,8 @@ sp-std = { version = "5.0.0", default-features = false, path = "../../../primiti [dev-dependencies] sp-io = { version = "7.0.0", path = "../../../primitives/io" } +sp-externalities = { version = "0.13.0", path = "../../../primitives/externalities" } +sp-version = { version = "5.0.0", path = "../../../primitives/version" } [features] default = ["std"] diff --git a/frame/system/benchmarking/res/README.md b/frame/system/benchmarking/res/README.md new file mode 100644 index 0000000000000..43bb2b5c283ef --- /dev/null +++ b/frame/system/benchmarking/res/README.md @@ -0,0 +1,5 @@ +These runtimes are used for benchmarking the `set_code` intrinsic. + +**Don't use them in production environments!** + +To update the just copy the new runtime from `target/release/wbuild/kitchensink-runtime/kitchensink_runtime.compact.compressed.wasm` to here. diff --git a/frame/system/benchmarking/res/kitchensink_runtime.compact.compressed.wasm b/frame/system/benchmarking/res/kitchensink_runtime.compact.compressed.wasm new file mode 100644 index 0000000000000..a0d2a4bb04b91 Binary files /dev/null and b/frame/system/benchmarking/res/kitchensink_runtime.compact.compressed.wasm differ diff --git a/frame/system/benchmarking/src/lib.rs b/frame/system/benchmarking/src/lib.rs index ef06729749422..1cd7b1bac6bd5 100644 --- a/frame/system/benchmarking/src/lib.rs +++ b/frame/system/benchmarking/src/lib.rs @@ -48,9 +48,12 @@ benchmarks! { set_heap_pages { }: _(RawOrigin::Root, Default::default()) - // `set_code` was not benchmarked because it is pretty hard to come up with a real - // Wasm runtime to test the upgrade with. But this is okay because we will make - // `set_code` take a full block anyway. + set_code { + let runtime_blob = include_bytes!("../res/kitchensink_runtime.compact.compressed.wasm").to_vec(); + }: _(RawOrigin::Root, runtime_blob) + verify { + System::::assert_last_event(frame_system::Event::::CodeUpdated.into()); + } #[extra] set_code_without_checks { diff --git a/frame/system/benchmarking/src/mock.rs b/frame/system/benchmarking/src/mock.rs index 8da623a3abbb5..8b05c5a8ba82a 100644 --- a/frame/system/benchmarking/src/mock.rs +++ b/frame/system/benchmarking/src/mock.rs @@ -19,6 +19,7 @@ #![cfg(test)] +use codec::Encode; use sp_runtime::traits::IdentityLookup; type AccountId = u64; @@ -67,7 +68,29 @@ impl frame_system::Config for Test { impl crate::Config for Test {} +struct MockedReadRuntimeVersion(Vec); + +impl sp_core::traits::ReadRuntimeVersion for MockedReadRuntimeVersion { + fn read_runtime_version( + &self, + _wasm_code: &[u8], + _ext: &mut dyn sp_externalities::Externalities, + ) -> Result, String> { + Ok(self.0.clone()) + } +} + pub fn new_test_ext() -> sp_io::TestExternalities { let t = frame_system::GenesisConfig::default().build_storage::().unwrap(); - sp_io::TestExternalities::new(t) + + let version = sp_version::RuntimeVersion { + spec_name: "spec_name".into(), + spec_version: 123, + impl_version: 456, + ..Default::default() + }; + let read_runtime_version = MockedReadRuntimeVersion(version.encode()); + let mut ext = sp_io::TestExternalities::new(t); + ext.register_extension(sp_core::traits::ReadRuntimeVersionExt::new(read_runtime_version)); + ext } diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index 81bead4f849b4..1b830105bb154 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -373,7 +373,6 @@ pub mod pallet { impl Pallet { /// Make some on-chain remark. /// - /// ## Complexity /// - `O(1)` #[pallet::call_index(0)] #[pallet::weight(T::SystemWeightInfo::remark(_remark.len() as u32))] @@ -393,31 +392,26 @@ pub mod pallet { } /// Set the new runtime code. - /// - /// ## Complexity - /// - `O(C + S)` where `C` length of `code` and `S` complexity of `can_set_code` #[pallet::call_index(2)] - #[pallet::weight((T::BlockWeights::get().max_block, DispatchClass::Operational))] + #[pallet::weight((T::SystemWeightInfo::set_code(), DispatchClass::Operational))] pub fn set_code(origin: OriginFor, code: Vec) -> DispatchResultWithPostInfo { ensure_root(origin)?; Self::can_set_code(&code)?; T::OnSetCode::set_code(code)?; - Ok(().into()) + // consume the rest of the block to prevent further transactions + Ok(Some(T::BlockWeights::get().max_block).into()) } /// Set the new runtime code without doing any checks of the given `code`. - /// - /// ## Complexity - /// - `O(C)` where `C` length of `code` #[pallet::call_index(3)] - #[pallet::weight((T::BlockWeights::get().max_block, DispatchClass::Operational))] + #[pallet::weight((T::SystemWeightInfo::set_code(), DispatchClass::Operational))] pub fn set_code_without_checks( origin: OriginFor, code: Vec, ) -> DispatchResultWithPostInfo { ensure_root(origin)?; T::OnSetCode::set_code(code)?; - Ok(().into()) + Ok(Some(T::BlockWeights::get().max_block).into()) } /// Set some items of storage. @@ -1414,9 +1408,6 @@ impl Pallet { } /// Deposits a log and ensures it matches the block's log data. - /// - /// ## Complexity - /// - `O(1)` pub fn deposit_log(item: generic::DigestItem) { >::append(item); } @@ -1622,15 +1613,23 @@ impl Pallet { .and_then(|v| RuntimeVersion::decode(&mut &v[..]).ok()) .ok_or(Error::::FailedToExtractRuntimeVersion)?; - if new_version.spec_name != current_version.spec_name { - return Err(Error::::InvalidSpecName.into()) - } + cfg_if::cfg_if! { + if #[cfg(all(feature = "runtime-benchmarks", not(test)))] { + // Let's ensure the compiler doesn't optimize our fetching of the runtime version away. + core::hint::black_box((new_version, current_version)); + Ok(()) + } else { + if new_version.spec_name != current_version.spec_name { + return Err(Error::::InvalidSpecName.into()) + } - if new_version.spec_version <= current_version.spec_version { - return Err(Error::::SpecVersionNeedsToIncrease.into()) - } + if new_version.spec_version <= current_version.spec_version { + return Err(Error::::SpecVersionNeedsToIncrease.into()) + } - Ok(()) + Ok(()) + } + } } } diff --git a/frame/system/src/tests.rs b/frame/system/src/tests.rs index 19b8d6a487ab5..128231ccf95cb 100644 --- a/frame/system/src/tests.rs +++ b/frame/system/src/tests.rs @@ -550,7 +550,12 @@ fn set_code_checks_works() { ("test", 1, 2, Err(Error::::SpecVersionNeedsToIncrease)), ("test", 1, 1, Err(Error::::SpecVersionNeedsToIncrease)), ("test2", 1, 1, Err(Error::::InvalidSpecName)), - ("test", 2, 1, Ok(PostDispatchInfo::default())), + ( + "test", + 2, + 1, + Ok(Some(::BlockWeights::get().max_block).into()), + ), ("test", 0, 1, Err(Error::::SpecVersionNeedsToIncrease)), ("test", 1, 0, Err(Error::::SpecVersionNeedsToIncrease)), ]; diff --git a/frame/system/src/weights.rs b/frame/system/src/weights.rs index 96fd7a1972eb2..64dce5356f196 100644 --- a/frame/system/src/weights.rs +++ b/frame/system/src/weights.rs @@ -18,25 +18,26 @@ //! Autogenerated weights for frame_system //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev -//! DATE: 2023-04-06, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2023-04-13, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` -//! HOSTNAME: `bm2`, CPU: `Intel(R) Core(TM) i7-7700K CPU @ 4.20GHz` +//! HOSTNAME: `bm3`, CPU: `Intel(R) Core(TM) i7-7700K CPU @ 4.20GHz` //! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024 // Executed Command: -// ./target/production/substrate +// target/production/substrate // benchmark // pallet -// --chain=dev // --steps=50 // --repeat=20 -// --pallet=frame_system // --extrinsic=* // --execution=wasm // --wasm-execution=compiled // --heap-pages=4096 -// --output=./frame/system/src/weights.rs +// --json-file=/var/lib/gitlab-runner/builds/zyw4fam_/0/parity/mirrors/substrate/.git/.artifacts/bench.json +// --pallet=frame_system +// --chain=dev // --header=./HEADER-APACHE2 +// --output=./frame/system/src/weights.rs // --template=./.maintain/frame-weight-template.hbs #![cfg_attr(rustfmt, rustfmt_skip)] @@ -44,13 +45,14 @@ #![allow(unused_imports)] use frame_support::{traits::Get, weights::{Weight, constants::RocksDbWeight}}; -use sp_std::marker::PhantomData; +use core::marker::PhantomData; /// Weight functions needed for frame_system. pub trait WeightInfo { fn remark(b: u32, ) -> Weight; fn remark_with_event(b: u32, ) -> Weight; fn set_heap_pages() -> Weight; + fn set_code() -> Weight; fn set_storage(i: u32, ) -> Weight; fn kill_storage(i: u32, ) -> Weight; fn kill_prefix(p: u32, ) -> Weight; @@ -64,20 +66,20 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `0` // Estimated: `0` - // Minimum execution time: 2_430_000 picoseconds. - Weight::from_parts(2_522_000, 0) + // Minimum execution time: 2_344_000 picoseconds. + Weight::from_parts(2_471_000, 0) // Standard Error: 0 - .saturating_add(Weight::from_parts(362, 0).saturating_mul(b.into())) + .saturating_add(Weight::from_parts(364, 0).saturating_mul(b.into())) } /// The range of component `b` is `[0, 3932160]`. fn remark_with_event(b: u32, ) -> Weight { // Proof Size summary in bytes: // Measured: `0` // Estimated: `0` - // Minimum execution time: 8_738_000 picoseconds. - Weight::from_parts(8_963_000, 0) + // Minimum execution time: 8_815_000 picoseconds. + Weight::from_parts(9_140_000, 0) // Standard Error: 0 - .saturating_add(Weight::from_parts(1_113, 0).saturating_mul(b.into())) + .saturating_add(Weight::from_parts(1_122, 0).saturating_mul(b.into())) } /// Storage: System Digest (r:1 w:1) /// Proof Skipped: System Digest (max_values: Some(1), max_size: None, mode: Measured) @@ -87,8 +89,21 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `0` // Estimated: `1485` - // Minimum execution time: 5_109_000 picoseconds. - Weight::from_parts(5_336_000, 1485) + // Minimum execution time: 5_233_000 picoseconds. + Weight::from_parts(5_462_000, 1485) + .saturating_add(T::DbWeight::get().reads(1_u64)) + .saturating_add(T::DbWeight::get().writes(2_u64)) + } + /// Storage: System Digest (r:1 w:1) + /// Proof Skipped: System Digest (max_values: Some(1), max_size: None, mode: Measured) + /// Storage: unknown `0x3a636f6465` (r:0 w:1) + /// Proof Skipped: unknown `0x3a636f6465` (r:0 w:1) + fn set_code() -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `1485` + // Minimum execution time: 58_606_683_000 picoseconds. + Weight::from_parts(59_115_121_000, 1485) .saturating_add(T::DbWeight::get().reads(1_u64)) .saturating_add(T::DbWeight::get().writes(2_u64)) } @@ -99,10 +114,10 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `0` // Estimated: `0` - // Minimum execution time: 2_531_000 picoseconds. - Weight::from_parts(2_617_000, 0) - // Standard Error: 861 - .saturating_add(Weight::from_parts(761_465, 0).saturating_mul(i.into())) + // Minimum execution time: 2_317_000 picoseconds. + Weight::from_parts(2_457_000, 0) + // Standard Error: 894 + .saturating_add(Weight::from_parts(750_850, 0).saturating_mul(i.into())) .saturating_add(T::DbWeight::get().writes((1_u64).saturating_mul(i.into()))) } /// Storage: Skipped Metadata (r:0 w:0) @@ -112,10 +127,10 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `0` // Estimated: `0` - // Minimum execution time: 2_631_000 picoseconds. - Weight::from_parts(2_717_000, 0) - // Standard Error: 1_097 - .saturating_add(Weight::from_parts(570_803, 0).saturating_mul(i.into())) + // Minimum execution time: 2_498_000 picoseconds. + Weight::from_parts(2_552_000, 0) + // Standard Error: 1_027 + .saturating_add(Weight::from_parts(566_064, 0).saturating_mul(i.into())) .saturating_add(T::DbWeight::get().writes((1_u64).saturating_mul(i.into()))) } /// Storage: Skipped Metadata (r:0 w:0) @@ -125,10 +140,10 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `116 + p * (69 ±0)` // Estimated: `121 + p * (70 ±0)` - // Minimum execution time: 4_692_000 picoseconds. - Weight::from_parts(4_789_000, 121) - // Standard Error: 1_013 - .saturating_add(Weight::from_parts(1_143_616, 0).saturating_mul(p.into())) + // Minimum execution time: 4_646_000 picoseconds. + Weight::from_parts(4_725_000, 121) + // Standard Error: 1_195 + .saturating_add(Weight::from_parts(1_144_884, 0).saturating_mul(p.into())) .saturating_add(T::DbWeight::get().reads((1_u64).saturating_mul(p.into()))) .saturating_add(T::DbWeight::get().writes((1_u64).saturating_mul(p.into()))) .saturating_add(Weight::from_parts(0, 70).saturating_mul(p.into())) @@ -142,20 +157,20 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `0` // Estimated: `0` - // Minimum execution time: 2_430_000 picoseconds. - Weight::from_parts(2_522_000, 0) + // Minimum execution time: 2_344_000 picoseconds. + Weight::from_parts(2_471_000, 0) // Standard Error: 0 - .saturating_add(Weight::from_parts(362, 0).saturating_mul(b.into())) + .saturating_add(Weight::from_parts(364, 0).saturating_mul(b.into())) } /// The range of component `b` is `[0, 3932160]`. fn remark_with_event(b: u32, ) -> Weight { // Proof Size summary in bytes: // Measured: `0` // Estimated: `0` - // Minimum execution time: 8_738_000 picoseconds. - Weight::from_parts(8_963_000, 0) + // Minimum execution time: 8_815_000 picoseconds. + Weight::from_parts(9_140_000, 0) // Standard Error: 0 - .saturating_add(Weight::from_parts(1_113, 0).saturating_mul(b.into())) + .saturating_add(Weight::from_parts(1_122, 0).saturating_mul(b.into())) } /// Storage: System Digest (r:1 w:1) /// Proof Skipped: System Digest (max_values: Some(1), max_size: None, mode: Measured) @@ -165,8 +180,21 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `0` // Estimated: `1485` - // Minimum execution time: 5_109_000 picoseconds. - Weight::from_parts(5_336_000, 1485) + // Minimum execution time: 5_233_000 picoseconds. + Weight::from_parts(5_462_000, 1485) + .saturating_add(RocksDbWeight::get().reads(1_u64)) + .saturating_add(RocksDbWeight::get().writes(2_u64)) + } + /// Storage: System Digest (r:1 w:1) + /// Proof Skipped: System Digest (max_values: Some(1), max_size: None, mode: Measured) + /// Storage: unknown `0x3a636f6465` (r:0 w:1) + /// Proof Skipped: unknown `0x3a636f6465` (r:0 w:1) + fn set_code() -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `1485` + // Minimum execution time: 58_606_683_000 picoseconds. + Weight::from_parts(59_115_121_000, 1485) .saturating_add(RocksDbWeight::get().reads(1_u64)) .saturating_add(RocksDbWeight::get().writes(2_u64)) } @@ -177,10 +205,10 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `0` // Estimated: `0` - // Minimum execution time: 2_531_000 picoseconds. - Weight::from_parts(2_617_000, 0) - // Standard Error: 861 - .saturating_add(Weight::from_parts(761_465, 0).saturating_mul(i.into())) + // Minimum execution time: 2_317_000 picoseconds. + Weight::from_parts(2_457_000, 0) + // Standard Error: 894 + .saturating_add(Weight::from_parts(750_850, 0).saturating_mul(i.into())) .saturating_add(RocksDbWeight::get().writes((1_u64).saturating_mul(i.into()))) } /// Storage: Skipped Metadata (r:0 w:0) @@ -190,10 +218,10 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `0` // Estimated: `0` - // Minimum execution time: 2_631_000 picoseconds. - Weight::from_parts(2_717_000, 0) - // Standard Error: 1_097 - .saturating_add(Weight::from_parts(570_803, 0).saturating_mul(i.into())) + // Minimum execution time: 2_498_000 picoseconds. + Weight::from_parts(2_552_000, 0) + // Standard Error: 1_027 + .saturating_add(Weight::from_parts(566_064, 0).saturating_mul(i.into())) .saturating_add(RocksDbWeight::get().writes((1_u64).saturating_mul(i.into()))) } /// Storage: Skipped Metadata (r:0 w:0) @@ -203,10 +231,10 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `116 + p * (69 ±0)` // Estimated: `121 + p * (70 ±0)` - // Minimum execution time: 4_692_000 picoseconds. - Weight::from_parts(4_789_000, 121) - // Standard Error: 1_013 - .saturating_add(Weight::from_parts(1_143_616, 0).saturating_mul(p.into())) + // Minimum execution time: 4_646_000 picoseconds. + Weight::from_parts(4_725_000, 121) + // Standard Error: 1_195 + .saturating_add(Weight::from_parts(1_144_884, 0).saturating_mul(p.into())) .saturating_add(RocksDbWeight::get().reads((1_u64).saturating_mul(p.into()))) .saturating_add(RocksDbWeight::get().writes((1_u64).saturating_mul(p.into()))) .saturating_add(Weight::from_parts(0, 70).saturating_mul(p.into())) diff --git a/frame/utility/src/tests.rs b/frame/utility/src/tests.rs index 6f156e318c8cf..ecc78ae6b17b6 100644 --- a/frame/utility/src/tests.rs +++ b/frame/utility/src/tests.rs @@ -170,7 +170,7 @@ impl frame_system::Config for Test { type AccountData = pallet_balances::AccountData; type OnNewAccount = (); type OnKilledAccount = (); - type SystemWeightInfo = (); + type SystemWeightInfo = frame_system::weights::SubstrateWeight; type SS58Prefix = (); type OnSetCode = (); type MaxConsumers = ConstU32<16>; @@ -916,12 +916,16 @@ fn batch_all_works_with_council_origin() { #[test] fn with_weight_works() { new_test_ext().execute_with(|| { + use frame_system::WeightInfo; let upgrade_code_call = Box::new(RuntimeCall::System(frame_system::Call::set_code_without_checks { code: vec![], })); // Weight before is max. - assert_eq!(upgrade_code_call.get_dispatch_info().weight, Weight::MAX); + assert_eq!( + upgrade_code_call.get_dispatch_info().weight, + ::SystemWeightInfo::set_code() + ); assert_eq!( upgrade_code_call.get_dispatch_info().class, frame_support::dispatch::DispatchClass::Operational