Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Fix benchmarks set_code for frame_system pallet for all SP runtimes #2765

Closed
bkontur opened this issue Jun 21, 2023 · 5 comments · Fixed by paritytech/substrate#14435
Closed
Labels
I3-bug The node fails to follow expected behavior.

Comments

@bkontur
Copy link
Contributor

bkontur commented Jun 21, 2023

here was added benchmark for set_code paritytech/substrate#13373
but it does not work with Cumulus runtimes which uses:

type OnSetCode = cumulus_pallet_parachain_system::ParachainSetCode<Self>;

because it ends here, where some data are expected
https://github.com/paritytech/cumulus/blob/master/pallets/parachain-system/src/lib.rs#L986-L1008

so the fix should be to add some T::BenchmarkHelper::prepare_data() to the set_code benchmark here

something like:

        set_code {
		let runtime_blob = include_bytes!("../res/kitchensink_runtime.compact.compressed.wasm").to_vec();
                T::BenchmarkHelper::prepare_data();
	}: _(RawOrigin::Root, runtime_blob)
	verify {
		System::<T>::assert_last_event(frame_system::Event::<T>::CodeUpdated.into());
	}

and then configure BenchmarkHelper in every runtime (or close to ParachainSetCode)

Example of failed job: https://gitlab.parity.io/parity/mirrors/cumulus/-/jobs/3035163

./target/production/polkadot-parachain benchmark pallet --chain=asset-hub-kusama-dev --execution=wasm --wasm-execution=compiled --pallet=frame_system --no-storage-info --no-median-slopes --no-min-squares '--extrinsic=*' --steps=50 --repeat=20 --json --header=./file_header.txt --output=./parachains/runtimes/assets/asset-hub-kusama/src/weights/
Error: Input("Benchmark frame_system::set_code failed: ValidationDataNotAvailable")
@bkontur bkontur changed the title Fix benchmarks for frame_system pallet for all SP runtimes Fix benchmarks set_code for frame_system pallet for all SP runtimes Jun 21, 2023
@bkchr
Copy link
Member

bkchr commented Jun 21, 2023

@bkontur will you work on this? Otherwise we could maybe for now work around by making ParachainSetCode be a no-op when runtime-benchmarks is enabled.

@bkchr bkchr added the I3-bug The node fails to follow expected behavior. label Jun 21, 2023
@bkontur
Copy link
Contributor Author

bkontur commented Jun 21, 2023

@bkontur will you work on this? Otherwise we could maybe for now work around by making ParachainSetCode be a no-op when runtime-benchmarks is enabled.

@bkchr do you mean something like this:

	/// The action to take on a Runtime Upgrade
	#[cfg(not(feature = "runtime-benchmarks"))]
	type OnSetCode = cumulus_pallet_parachain_system::ParachainSetCode<Self>;
	#[cfg(feature = "runtime-benchmarks")]
	type OnSetCode = ();

or

impl<T: Config> frame_system::SetCode<T> for ParachainSetCode<T> {
	#[cfg(not(feature = "runtime-benchmarks"))]
	fn set_code(code: Vec<u8>) -> DispatchResult {
		Pallet::<T>::schedule_code_upgrade(code)
		Ok(())
	}

	#[cfg(feature = "runtime-benchmarks")]
	fn set_code(code: Vec<u8>) -> DispatchResult {
		Ok(())
	}
}

for () I see it does at least <Pallet<T>>::update_code_in_storage(&code)?;

@bkchr
Copy link
Member

bkchr commented Jun 21, 2023

Option 2, but this being just some intermediate solution and the real one being what you have proposed in the issue description.

@bkontur
Copy link
Contributor Author

bkontur commented Jun 21, 2023

Option 2, but this being just some intermediate solution and the real one being what you have proposed in the issue description.

yes, here is tmp solution: #2766
and I will start on real proposed one, it will just require polkadot and cumulus companions

@bkontur
Copy link
Contributor Author

bkontur commented Jun 22, 2023

@bkchr
so no temporary, I am done, locally it is working finally,
paritytech/substrate#14435
#2766

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
I3-bug The node fails to follow expected behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants