difftreelog
fix(preimage-execution) introduce weight expectation + adjust benchmarks + new tests
in: master
6 files changed
pallets/maintenance/src/benchmarking.rsdiffbeforeafterboth191920use frame_benchmarking::benchmarks;20use frame_benchmarking::benchmarks;21use frame_system::{Call as RuntimeCall, RawOrigin};21use frame_system::{Call as RuntimeCall, RawOrigin};22use frame_support::{ensure, traits::StorePreimage};22use frame_support::{ensure, pallet_prelude::Weight, traits::StorePreimage};23use codec::Encode;23use codec::Encode;24use sp_std::vec;24use sp_std::vec;252540 execute_preimage {40 execute_preimage {41 let call_hash = RuntimeCall::<T>::set_storage { items: vec![] }.encode();41 let call_hash = RuntimeCall::<T>::set_storage { items: vec![] }.encode();42 let hash = T::Preimages::note(call_hash.into())?;42 let hash = T::Preimages::note(call_hash.into())?;43 }: _(RawOrigin::Root, hash)43 }: _(RawOrigin::Root, hash, None, Weight::from_parts(100000000000, 100000000000))44 verify {44 verify {45 }45 }46}46}pallets/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,
+ }),
}
}
}
runtime/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 {
runtime/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);
tests/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;
tests/src/util/identitySetter.tsdiffbeforeafterboth--- a/tests/src/util/identitySetter.ts
+++ b/tests/src/util/identitySetter.ts
@@ -172,8 +172,6 @@
// identitiesToRemove.push((key as any).toHuman()[0]);
}
- console.log(identitiesToAdd[0][1]);
-
if (identitiesToRemove.length != 0)
await uploadPreimage(
helper,