Run benchmarks and update weights for pallet_collator_selection once benchmarks are fixed for external pallets

shannonwells commented 1 year ago

Update benchmarks to use the proper local Benchmarks when this is working

[x] pallet_collator_selection - this should be fixed in cumulus branch polkadot-v0.9.31.

Todo

[x] Add it to the list of benchmarks to run scripts/run_all_benchmarks.sh
[x] Add the generated file to runtime/common/src/weights/mod.rs
[x] Run the new benchmarks on the reference hardware

p5150j commented 1 year ago

on hold, revisit after 0.9.36 upgrade

rlaferla commented 1 year ago

Even with 0.9.36, the benchmarks fail. I am researching the error that is causing unwrap() to panic.

2023-01-26 12:57:20 Starting benchmark: pallet_collator_selection::register_as_candidate    
2023-01-26 12:57:20 panicked at 'called `Result::unwrap()` on an `Err` value: <wasm:stripped>', /workarea/.cargo/git/checkouts/cumulus-59522f43471fa161/afe528a/pallets/collator-selection/src/benchmarking.rs:103:86    
Error: Input("Error executing and verifying runtime benchmark: Execution aborted due to trap: wasm trap: wasm `unreachable` instruction executed\nWASM backtrace:\n\n    0: 0x38c7 - <unknown>!rust_begin_unwind\n    1: 0x1b3e - <unknown>!core::panicking::panic_fmt::h57b56b1dc717ba48\n    2: 0x2208 - <unknown>!core::result::unwrap_failed::h4020e4ec86ef2ed8\n    3: 0x16c137 - <unknown>!pallet_collator_selection::benchmarking::register_candidates::hbdbf72fa3adb997a\n    4: 0x1488d6 - <unknown>!<frequency_runtime::Runtime as frame_benchmarking::utils::runtime_decl_for_Benchmark::BenchmarkV1<sp_runtime::generic::block::Block<sp_runtime::generic::header::Header<u32,sp_runtime::traits::BlakeTwo256>,sp_runtime::generic::unchecked_extrinsic::UncheckedExtrinsic<sp_runtime::multiaddress::MultiAddress<<<sp_runtime::MultiSignature as sp_runtime::traits::Verify>::Signer as sp_runtime::traits::IdentifyAccount>::AccountId,()>,frequency_runtime::RuntimeCall,sp_runtime::MultiSignature,(frame_system::extensions::check_non_zero_sender::CheckNonZeroSender<frequency_runtime::Runtime>,frame_system::extensions::check_spec_version::CheckSpecVersion<frequency_runtime::Runtime>,frame_system::extensions::check_tx_version::CheckTxVersion<frequency_runtime::Runtime>,frame_system::extensions::check_genesis::CheckGenesis<frequency_runtime::Runtime>,frame_system::extensions::check_mortality::CheckMortality<frequency_runtime::Runtime>,common_runtime::extensions::check_nonce::CheckNonce<frequency_runtime::Runtime>,frame_system::extensions::check_weight::CheckWeight<frequency_runtime::Runtime>,pallet_transaction_payment::ChargeTransactionPayment<frequency_runtime::Runtime>,pallet_msa::CheckFreeExtrinsicUse<frequency_runtime::Runtime>)>>>>::dispatch_benchmark::ha1e526a41c28b52f\n    5: 0x1b10cc - <unknown>!Benchmark_dispatch_benchmark\n")
❌ 💔

rlaferla commented 1 year ago

The problem stems from the default values of 0 for CollatorMinCandidates and CollatorMaxCandidates for each of the three environments (local, testnet, mainnet.) It is unclear what these values should be. The benchmark panics when they are set to 0 when it should have an assert to test for invalid configurations. I may file an issue with Parity for it.

https://paritytech.github.io/cumulus/pallet_collator_selection/index.html

rlaferla commented 1 year ago

After consulting with @wilwade , it was decided to make CollatorMinCandidates = 1 and CollatorMaxCandidates = 50

frequency-chain / frequency

Run benchmarks and update weights for pallet_collator_selection once benchmarks are fixed for external pallets #608

Todo