paritytech / project-mythical

Mythos parachain node.
The Unlicense
4 stars 0 forks source link

[Medium] External pallets should be benchmarked with the accurate runtime #92

Closed kevin-valerio closed 5 months ago

kevin-valerio commented 5 months ago

Summary

Mythical depends on a subset of FRAME pallets. The benchmarks for these pallets are done using external runtimes instead of the correct Mythical runtimes (testnet and mainnet).

Issue details

Mythical relies on weights for their FRAME pallet dependencies that are benchmarked with some external runtimes instead of the actual runtime.

You can find below an example of an incorrect benchmark for pallet_balances using Substrate-native pallets:

impl pallet_balances::Config for Runtime {
    ...
    type WeightInfo = pallet_balances::weights::SubstrateWeight<Runtime>;
}

So far, this issue has been spotted for the following pallets :

Substrate-native imported benchmarks

Risk

As pallet extrinsic benchmarks can be dependent on the actual runtime configuration, this can lead to:

for all extrinsics that are using the substrate-node template runtime weights.

Mitigation

All pallet extrinsics, even the Substrate ones, should be benchmarked with the actual runtime configuration by including them in your define_benchmarks! block.

A best practice example can be found in the Kusama runtime implementation.

Moliholy commented 5 months ago

@kevin-valerio This has already been addressed in #100, #103 and #106.

redzsina commented 5 months ago

Fix verified, closing issue