git.delta.rocks / unique-network / refs/commits / 6ec8a32fcc0b

difftreelog

refactor(preimage-execution) remove preimage length bound + fix benchmarks

Fahrrader2023-02-22parent: #1ae086c.patch.diff
in: master

7 files changed

modifiedMakefilediffbeforeafterboth
--- a/Makefile
+++ b/Makefile
@@ -137,9 +137,13 @@
 bench-app-promotion:
 	make _bench PALLET=app-promotion PALLET_DIR=app-promotion
 
+.PHONY: bench-maintenance
+bench-maintenance:
+	make _bench PALLET=maintenance
+
 .PHONY: bench
 # Disabled: bench-scheduler, bench-collator-selection, bench-identity
-bench: bench-common bench-evm-migration bench-unique bench-structure bench-fungible bench-refungible bench-nonfungible bench-configuration bench-foreign-assets
+bench: bench-common bench-evm-migration bench-unique bench-structure bench-fungible bench-refungible bench-nonfungible bench-configuration bench-foreign-assets bench-maintenance
 
 .PHONY: check
 check:
modifiedpallets/maintenance/src/benchmarking.rsdiffbeforeafterboth
--- a/pallets/maintenance/src/benchmarking.rs
+++ b/pallets/maintenance/src/benchmarking.rs
@@ -17,11 +17,10 @@
 use super::*;
 use crate::{Pallet as Maintenance, Config};
 
+use codec::Encode;
 use frame_benchmarking::benchmarks;
-use frame_system::{Call as RuntimeCall, RawOrigin};
+use frame_system::RawOrigin;
 use frame_support::{ensure, pallet_prelude::Weight, traits::StorePreimage};
-use codec::Encode;
-use sp_std::vec;
 
 benchmarks! {
 	enable {
@@ -38,9 +37,9 @@
 	}
 
 	execute_preimage {
-		let call_hash = RuntimeCall::<T>::set_storage { items: vec![] }.encode();
-		let hash = T::Preimages::note(call_hash.into())?;
-	}: _(RawOrigin::Root, hash, None, Weight::from_parts(100000000000, 100000000000))
+		let call = <T as Config>::RuntimeCall::from(frame_system::Call::<T>::remark { remark: 1u32.encode() });
+		let hash = T::Preimages::note(call.encode().into())?;
+	}: _(RawOrigin::Root, hash, Weight::from_parts(100000000000, 100000000000))
 	verify {
 	}
 }
modifiedpallets/maintenance/src/lib.rsdiffbeforeafterboth
--- a/pallets/maintenance/src/lib.rs
+++ b/pallets/maintenance/src/lib.rs
@@ -102,19 +102,22 @@
 			Ok(())
 		}
 
+		/// Execute a runtime call stored as a preimage.
+		///
+		/// `weight_bound` is the maximum weight that the caller is willing
+		/// to allow the extrinsic to be executed with.
 		#[pallet::call_index(2)]
 		#[pallet::weight(<T as Config>::WeightInfo::execute_preimage())]
 		pub fn execute_preimage(
 			origin: OriginFor<T>,
 			hash: H256,
-			preimage_length: Option<u32>,
 			weight_bound: Weight,
 		) -> DispatchResultWithPostInfo {
 			use codec::Decode;
 
 			ensure_root(origin)?;
 
-			let data = T::Preimages::fetch(&hash, preimage_length)?;
+			let data = T::Preimages::fetch(&hash, None)?;
 			weight_bound.set_proof_size(
 				weight_bound
 					.proof_size()
@@ -135,15 +138,9 @@
 			);
 
 			match call.dispatch(frame_system::RawOrigin::Root.into()) {
-				Ok(post_info) => Ok(PostDispatchInfo {
-					actual_weight: post_info.actual_weight,
-					pays_fee: Pays::No,
-				}),
+				Ok(_) => Ok(Pays::No.into()),
 				Err(error_and_info) => Err(DispatchErrorWithPostInfo {
-					post_info: PostDispatchInfo {
-						actual_weight: error_and_info.post_info.actual_weight,
-						pays_fee: Pays::No,
-					},
+					post_info: Pays::No.into(),
 					error: error_and_info.error,
 				}),
 			}
modifiedpallets/maintenance/src/weights.rsdiffbeforeafterboth
before · pallets/maintenance/src/weights.rs
1// Template adopted from https://github.com/paritytech/substrate/blob/master/.maintain/frame-weight-template.hbs23//! Autogenerated weights for pallet_maintenance4//!5//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev6//! DATE: 2022-11-01, STEPS: `50`, REPEAT: 80, LOW RANGE: `[]`, HIGH RANGE: `[]`7//! EXECUTION: None, WASM-EXECUTION: Compiled, CHAIN: None, DB CACHE: 102489// Executed Command:10// target/release/unique-collator11// benchmark12// pallet13// --pallet14// pallet-maintenance15// --wasm-execution16// compiled17// --extrinsic18// *19// --template20// .maintain/frame-weight-template.hbs21// --steps=5022// --repeat=8023// --heap-pages=409624// --output=./pallets/maintenance/src/weights.rs2526#![cfg_attr(rustfmt, rustfmt_skip)]27#![allow(unused_parens)]28#![allow(unused_imports)]29#![allow(clippy::unnecessary_cast)]3031use frame_support::{traits::Get, weights::{Weight, constants::RocksDbWeight}};32use sp_std::marker::PhantomData;3334/// Weight functions needed for pallet_maintenance.35pub trait WeightInfo {36	fn enable() -> Weight;37	fn disable() -> Weight;38	fn execute_preimage() -> Weight;39}4041/// Weights for pallet_maintenance using the Substrate node and recommended hardware.42pub struct SubstrateWeight<T>(PhantomData<T>);43impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {44	// Storage: Maintenance Enabled (r:0 w:1)45	fn enable() -> Weight {46		Weight::from_ref_time(7_367_000)47			.saturating_add(T::DbWeight::get().writes(1))48	}49	// Storage: Maintenance Enabled (r:0 w:1)50	fn disable() -> Weight {51		Weight::from_ref_time(7_273_000)52			.saturating_add(T::DbWeight::get().writes(1))53	}54	// Storage: 55	fn execute_preimage() -> Weight {56		Weight::from_ref_time(7_273_000)57			.saturating_add(T::DbWeight::get().reads(2))58			.saturating_add(T::DbWeight::get().writes(1))59	}60}6162// For backwards compatibility and tests63impl WeightInfo for () {64	// Storage: Maintenance Enabled (r:0 w:1)65	fn enable() -> Weight {66		Weight::from_ref_time(7_367_000)67			.saturating_add(RocksDbWeight::get().writes(1))68	}69	// Storage: Maintenance Enabled (r:0 w:1)70	fn disable() -> Weight {71		Weight::from_ref_time(7_273_000)72			.saturating_add(RocksDbWeight::get().writes(1))73	}74	// Storage: 75	fn execute_preimage() -> Weight {76		Weight::from_ref_time(7_273_000)77			.saturating_add(RocksDbWeight::get().reads(2))78			.saturating_add(RocksDbWeight::get().writes(1))79	}80}
after · pallets/maintenance/src/weights.rs
1// Template adopted from https://github.com/paritytech/substrate/blob/master/.maintain/frame-weight-template.hbs23//! Autogenerated weights for pallet_maintenance4//!5//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev6//! DATE: 2023-02-22, STEPS: `50`, REPEAT: 80, LOW RANGE: `[]`, HIGH RANGE: `[]`7//! EXECUTION: None, WASM-EXECUTION: Compiled, CHAIN: None, DB CACHE: 102489// Executed Command:10// target/release/unique-collator11// benchmark12// pallet13// --pallet14// pallet-maintenance15// --wasm-execution16// compiled17// --extrinsic18// *19// --template20// .maintain/frame-weight-template.hbs21// --steps=5022// --repeat=8023// --heap-pages=409624// --output=./pallets/maintenance/src/weights.rs2526#![cfg_attr(rustfmt, rustfmt_skip)]27#![allow(unused_parens)]28#![allow(unused_imports)]29#![allow(missing_docs)]30#![allow(clippy::unnecessary_cast)]3132use frame_support::{traits::Get, weights::{Weight, constants::RocksDbWeight}};33use sp_std::marker::PhantomData;3435/// Weight functions needed for pallet_maintenance.36pub trait WeightInfo {37	fn enable() -> Weight;38	fn disable() -> Weight;39	fn execute_preimage() -> Weight;40}4142/// Weights for pallet_maintenance using the Substrate node and recommended hardware.43pub struct SubstrateWeight<T>(PhantomData<T>);44impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {45	// Storage: Maintenance Enabled (r:0 w:1)46	fn enable() -> Weight {47		Weight::from_ref_time(10_860_000 as u64)48			.saturating_add(T::DbWeight::get().writes(1 as u64))49	}50	// Storage: Maintenance Enabled (r:0 w:1)51	fn disable() -> Weight {52		Weight::from_ref_time(10_871_000 as u64)53			.saturating_add(T::DbWeight::get().writes(1 as u64))54	}55	// Storage: Preimage StatusFor (r:1 w:0)56	// Storage: Preimage PreimageFor (r:1 w:0)57	fn execute_preimage() -> Weight {58		Weight::from_ref_time(10_068_000 as u64)59			.saturating_add(T::DbWeight::get().reads(2 as u64))60	}61}6263// For backwards compatibility and tests64impl WeightInfo for () {65	// Storage: Maintenance Enabled (r:0 w:1)66	fn enable() -> Weight {67		Weight::from_ref_time(10_860_000 as u64)68			.saturating_add(RocksDbWeight::get().writes(1 as u64))69	}70	// Storage: Maintenance Enabled (r:0 w:1)71	fn disable() -> Weight {72		Weight::from_ref_time(10_871_000 as u64)73			.saturating_add(RocksDbWeight::get().writes(1 as u64))74	}75	// Storage: Preimage StatusFor (r:1 w:0)76	// Storage: Preimage PreimageFor (r:1 w:0)77	fn execute_preimage() -> Weight {78		Weight::from_ref_time(10_068_000 as u64)79			.saturating_add(RocksDbWeight::get().reads(2 as u64))80	}81}
modifiedruntime/common/maintenance.rsdiffbeforeafterboth
--- a/runtime/common/maintenance.rs
+++ b/runtime/common/maintenance.rs
@@ -86,9 +86,6 @@
 				| RuntimeCall::Session(_)
 				| RuntimeCall::Identity(_) => Err(TransactionValidityError::Invalid(InvalidTransaction::Call)),
 
-				#[cfg(feature = "preimage")]
-				RuntimeCall::Preimage(_) => Err(TransactionValidityError::Invalid(InvalidTransaction::Call)),
-
 				#[cfg(feature = "pallet-test-utils")]
 				RuntimeCall::TestUtils(_) => Err(TransactionValidityError::Invalid(InvalidTransaction::Call)),
 
modifiedruntime/common/runtime_apis.rsdiffbeforeafterboth
--- a/runtime/common/runtime_apis.rs
+++ b/runtime/common/runtime_apis.rs
@@ -568,6 +568,7 @@
                     #[cfg(feature = "foreign-assets")]
                     list_benchmark!(list, extra, pallet_foreign_assets, ForeignAssets);
 
+                    list_benchmark!(list, extra, pallet_maintenance, Maintenance);
 
                     // list_benchmark!(list, extra, pallet_evm_coder_substrate, EvmCoderSubstrate);
 
@@ -632,6 +633,8 @@
                     #[cfg(feature = "foreign-assets")]
                     add_benchmark!(params, batches, pallet_foreign_assets, ForeignAssets);
 
+                    add_benchmark!(params, batches, pallet_maintenance, Maintenance);
+
                     // add_benchmark!(params, batches, pallet_evm_coder_substrate, EvmCoderSubstrate);
 
                     if batches.is_empty() { return Err("Benchmark not found for this pallet.".into()) }
modifiedtests/src/maintenance.seqtest.tsdiffbeforeafterboth
--- a/tests/src/maintenance.seqtest.ts
+++ b/tests/src/maintenance.seqtest.ts
@@ -279,7 +279,7 @@
   });
 
   describe('Preimage Execution', () => {
-    let preimageHash: string;
+    const preimageHashes: string[] = [];
 
     async function notePreimage(helper: UniqueHelper, preimage: any): Promise<string> {
       const result = await helper.preimage.notePreimage(bob, preimage);
@@ -306,13 +306,13 @@
           },
         ]);
         const preimage = helper.constructApiCall('api.tx.identity.forceInsertIdentities', [randomIdentities]).method.toHex();
-        preimageHash = await notePreimage(helper, preimage);
+        preimageHashes.push(await notePreimage(helper, preimage));
       });
     });
 
     itSub('Successfully executes call in a preimage', async ({helper}) => {
       const result = await expect(helper.getSudo().executeExtrinsic(superuser, 'api.tx.maintenance.executePreimage', [
-        preimageHash, null, {refTime: 10000000000, proofSize: 10000000000},
+        preimageHashes[0], {refTime: 10000000000, proofSize: 10000000000},
       ])).to.be.fulfilled;
 
       // preimage is executed, and an appropriate event is present
@@ -320,7 +320,7 @@
       expect(events.length).to.be.equal(1);
 
       // the preimage goes back to being unrequested
-      expect(await helper.preimage.getPreimageInfo(preimageHash)).to.have.property('unrequested');
+      expect(await helper.preimage.getPreimageInfo(preimageHashes[0])).to.have.property('unrequested');
     });
 
     itSub('Does not allow execution of a preimage that would fail', async ({helper}) => {
@@ -330,27 +330,28 @@
         {Id: zeroAccount.address}, {Id: superuser.address}, 1000n,
       ]).method.toHex();
       const preimageHash = await notePreimage(helper, preimage);
+      preimageHashes.push(preimageHash);
 
       await expect(helper.getSudo().executeExtrinsic(superuser, 'api.tx.maintenance.executePreimage', [
-        preimageHash, null, {refTime: 100000000000, proofSize: 100000000000},
+        preimageHash, {refTime: 100000000000, proofSize: 100000000000},
       ])).to.be.rejectedWith(/balances\.InsufficientBalance/);
     });
 
     itSub('Does not allow preimage execution with non-root', async ({helper}) => {
       await expect(helper.executeExtrinsic(bob, 'api.tx.maintenance.executePreimage', [
-        preimageHash, null, {refTime: 100000000000, proofSize: 100000000000},
+        preimageHashes[0], {refTime: 100000000000, proofSize: 100000000000},
       ])).to.be.rejectedWith(/BadOrigin/);
     });
 
     itSub('Does not allow execution of non-existent preimages', async ({helper}) => {
       await expect(helper.getSudo().executeExtrinsic(superuser, 'api.tx.maintenance.executePreimage', [
-        '0x1010101010101010101010101010101010101010101010101010101010101010', null, {refTime: 100000000000, proofSize: 100000000000},
+        '0x1010101010101010101010101010101010101010101010101010101010101010', {refTime: 100000000000, proofSize: 100000000000},
       ])).to.be.rejectedWith(/Unavailable/);
     });
 
     itSub('Does not allow preimage execution with less than minimum weights', async ({helper}) => {
       await expect(helper.getSudo().executeExtrinsic(superuser, 'api.tx.maintenance.executePreimage', [
-        preimageHash, null, {refTime: 1000, proofSize: 1000},
+        preimageHashes[0], {refTime: 1000, proofSize: 1000},
       ])).to.be.rejectedWith(/Exhausted/);
     });
 
@@ -358,7 +359,9 @@
       await usingPlaygrounds(async (helper) => {
         if (helper.fetchMissingPalletNames([Pallets.Preimage, Pallets.Maintenance]).length != 0) return;
 
-        await helper.preimage.unnotePreimage(bob, preimageHash);
+        for (const hash of preimageHashes) {
+          await helper.preimage.unnotePreimage(bob, hash);
+        }
       });
     });
   });