git.delta.rocks / unique-network / refs/commits / 1ae086c66ec9

difftreelog

fix(preimage-execution) introduce weight expectation + adjust benchmarks + new tests

Fahrrader2023-02-22parent: #23641e0.patch.diff
in: master

6 files changed

modifiedpallets/maintenance/src/benchmarking.rsdiffbeforeafterboth
--- a/pallets/maintenance/src/benchmarking.rs
+++ b/pallets/maintenance/src/benchmarking.rs
@@ -19,7 +19,7 @@
 
 use frame_benchmarking::benchmarks;
 use frame_system::{Call as RuntimeCall, RawOrigin};
-use frame_support::{ensure, traits::StorePreimage};
+use frame_support::{ensure, pallet_prelude::Weight, traits::StorePreimage};
 use codec::Encode;
 use sp_std::vec;
 
@@ -40,7 +40,7 @@
 	execute_preimage {
 		let call_hash = RuntimeCall::<T>::set_storage { items: vec![] }.encode();
 		let hash = T::Preimages::note(call_hash.into())?;
-	}: _(RawOrigin::Root, hash)
+	}: _(RawOrigin::Root, hash, None, Weight::from_parts(100000000000, 100000000000))
 	verify {
 	}
 }
modifiedpallets/maintenance/src/lib.rsdiffbeforeafterboth
--- a/pallets/maintenance/src/lib.rs
+++ b/pallets/maintenance/src/lib.rs
@@ -104,30 +104,48 @@
 
 		#[pallet::call_index(2)]
 		#[pallet::weight(<T as Config>::WeightInfo::execute_preimage())]
-		pub fn execute_preimage(_origin: OriginFor<T>, _hash: H256) -> DispatchResult {
-			#[cfg(feature = "preimage")]
-			{
-				let origin = _origin;
-				let hash = _hash;
+		pub fn execute_preimage(
+			origin: OriginFor<T>,
+			hash: H256,
+			preimage_length: Option<u32>,
+			weight_bound: Weight,
+		) -> DispatchResultWithPostInfo {
+			use codec::Decode;
 
-				ensure_root(origin)?;
+			ensure_root(origin)?;
 
-				let len = T::Preimages::len(&hash).ok_or(DispatchError::Unavailable)?;
-				let bounded = T::Preimages::pick::<<T as Config>::RuntimeCall>(hash, len);
-				let (call, _) =
-					T::Preimages::realize(&bounded).map_err(|_| DispatchError::Unavailable)?;
+			let data = T::Preimages::fetch(&hash, preimage_length)?;
+			weight_bound.set_proof_size(
+				weight_bound
+					.proof_size()
+					.checked_sub(
+						data.len()
+							.try_into()
+							.map_err(|_| DispatchError::Corruption)?,
+					)
+					.ok_or(DispatchError::Exhausted)?,
+			);
 
-				let result = match call.dispatch(frame_system::RawOrigin::Root.into()) {
-					Ok(_) => Ok(()),
-					Err(error_and_info) => Err(error_and_info.error),
-				};
+			let call = <T as Config>::RuntimeCall::decode(&mut &data[..])
+				.map_err(|_| DispatchError::Corruption)?;
 
-				result
-			}
+			ensure!(
+				call.get_dispatch_info().weight.all_lte(weight_bound),
+				DispatchError::Exhausted
+			);
 
-			#[cfg(not(feature = "preimage"))]
-			{
-				Err(DispatchError::Unavailable)
+			match call.dispatch(frame_system::RawOrigin::Root.into()) {
+				Ok(post_info) => Ok(PostDispatchInfo {
+					actual_weight: post_info.actual_weight,
+					pays_fee: Pays::No,
+				}),
+				Err(error_and_info) => Err(DispatchErrorWithPostInfo {
+					post_info: PostDispatchInfo {
+						actual_weight: error_and_info.post_info.actual_weight,
+						pays_fee: Pays::No,
+					},
+					error: error_and_info.error,
+				}),
 			}
 		}
 	}
modifiedruntime/common/config/pallets/preimage.rsdiffbeforeafterboth
--- a/runtime/common/config/pallets/preimage.rs
+++ b/runtime/common/config/pallets/preimage.rs
@@ -20,8 +20,7 @@
 use up_common::constants::*;
 
 parameter_types! {
-	pub PreimageBaseDeposit: Balance = 1000 * UNIQUE; // deposit(2, 64);
-	// pub PreimageByteDeposit: Balance = 1 * CENTIUNIQUE; // deposit(0, 1);
+	pub PreimageBaseDeposit: Balance = 1000 * UNIQUE;
 }
 
 impl pallet_preimage::Config for Runtime {
modifiedruntime/common/runtime_apis.rsdiffbeforeafterboth
--- a/runtime/common/runtime_apis.rs
+++ b/runtime/common/runtime_apis.rs
@@ -565,9 +565,6 @@
                     #[cfg(feature = "collator-selection")]
                     list_benchmark!(list, extra, pallet_identity, Identity);
 
-                    #[cfg(feature = "preimage")]
-                    list_benchmark!(list, extra, pallet_preimage, Preimage);
-
                     #[cfg(feature = "foreign-assets")]
                     list_benchmark!(list, extra, pallet_foreign_assets, ForeignAssets);
 
@@ -631,9 +628,6 @@
 
                     #[cfg(feature = "collator-selection")]
                     add_benchmark!(params, batches, pallet_identity, Identity);
-
-                    #[cfg(feature = "preimage")]
-                    add_benchmark!(params, batches, pallet_preimage, Preimage);
 
                     #[cfg(feature = "foreign-assets")]
                     add_benchmark!(params, batches, pallet_foreign_assets, ForeignAssets);
modifiedtests/src/maintenance.seqtest.tsdiffbeforeafterboth
--- a/tests/src/maintenance.seqtest.ts
+++ b/tests/src/maintenance.seqtest.ts
@@ -18,6 +18,7 @@
 import {ApiPromise} from '@polkadot/api';
 import {expect, itSched, itSub, Pallets, requirePalletsOrSkip, usingPlaygrounds} from './util';
 import {itEth} from './eth/util';
+import {UniqueHelper} from './util/playgrounds/unique';
 
 async function maintenanceEnabled(api: ApiPromise): Promise<boolean> {
   return (await api.query.maintenance.enabled()).toJSON() as boolean;
@@ -280,6 +281,13 @@
   describe('Preimage Execution', () => {
     let preimageHash: string;
 
+    async function notePreimage(helper: UniqueHelper, preimage: any): Promise<string> {
+      const result = await helper.preimage.notePreimage(bob, preimage);
+      const events = result.result.events.filter(x => x.event.method === 'Noted' && x.event.section === 'preimage');
+      const preimageHash = events[0].event.data[0].toHuman();
+      return preimageHash;
+    }
+
     before(async function() {
       await usingPlaygrounds(async (helper) => {
         requirePalletsOrSkip(this, helper, [Pallets.Preimage, Pallets.Maintenance]);
@@ -298,15 +306,14 @@
           },
         ]);
         const preimage = helper.constructApiCall('api.tx.identity.forceInsertIdentities', [randomIdentities]).method.toHex();
-        const result = await helper.preimage.notePreimage(bob, preimage);
-        const events = result.result.events.filter(x => x.event.method === 'Noted' && x.event.section === 'preimage');
-        preimageHash = events[0].event.data[0].toHuman();
+        preimageHash = 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]))
-        .to.be.fulfilled;
+      const result = await expect(helper.getSudo().executeExtrinsic(superuser, 'api.tx.maintenance.executePreimage', [
+        preimageHash, null, {refTime: 10000000000, proofSize: 10000000000},
+      ])).to.be.fulfilled;
 
       // preimage is executed, and an appropriate event is present
       const events = result.result.events.filter((x: any) => x.event.method === 'IdentitiesInserted' && x.event.section === 'identity');
@@ -316,17 +323,37 @@
       expect(await helper.preimage.getPreimageInfo(preimageHash)).to.have.property('unrequested');
     });
 
+    itSub('Does not allow execution of a preimage that would fail', async ({helper}) => {
+      const [zeroAccount] = await helper.arrange.createAccounts([0n], superuser);
+
+      const preimage = helper.constructApiCall('api.tx.balances.forceTransfer', [
+        {Id: zeroAccount.address}, {Id: superuser.address}, 1000n,
+      ]).method.toHex();
+      const preimageHash = await notePreimage(helper, preimage);
+
+      await expect(helper.getSudo().executeExtrinsic(superuser, 'api.tx.maintenance.executePreimage', [
+        preimageHash, null, {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]))
-        .to.be.rejectedWith(/BadOrigin/);
+      await expect(helper.executeExtrinsic(bob, 'api.tx.maintenance.executePreimage', [
+        preimageHash, null, {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',
+        '0x1010101010101010101010101010101010101010101010101010101010101010', null, {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},
+      ])).to.be.rejectedWith(/Exhausted/);
+    });
+
     after(async function() {
       await usingPlaygrounds(async (helper) => {
         if (helper.fetchMissingPalletNames([Pallets.Preimage, Pallets.Maintenance]).length != 0) return;
modifiedtests/src/util/identitySetter.tsdiffbeforeafterboth
172 // identitiesToRemove.push((key as any).toHuman()[0]);172 // identitiesToRemove.push((key as any).toHuman()[0]);
173 }173 }
174
175 console.log(identitiesToAdd[0][1]);
176174
177 if (identitiesToRemove.length != 0)175 if (identitiesToRemove.length != 0)
178 await uploadPreimage(176 await uploadPreimage(