Error in some pallets when attempting to update weights with the benchmarking tool.

[s3krit]

Description

When attempting to update the benchmarks for polkadot, kusama and westend, the binary crashes with Thread 'main' panicked at 'called Option::unwrap()on aNone value', /home/martin/.cargo/git/checkouts/substrate-7e08433d4c370a21/f9d9382/utils/frame/benchmarking-cli/src/writer.rs:152

Expected behaviour

When executing the benchmarking subcommand with --output=./some/runtime/weights.rs, the benchmarks are run and the weights are updated for the specified pallet and extrinsic.

Observed behaviour

When not specifying an output directory, the benchmark completes with a summary. When specifying an output file/directory, as demonstrated below, for some pallets I receive the following error:

martin@bm2:~/polkadot$ cargo run --release --features=runtime-benchmarks -- benchmark --chain=dev --pallet=pallet_balances --extrinsic="*" --steps=50 --repeat=20 --output=.
    Finished release [optimized] target(s) in 0.34s
     Running `target/release/polkadot benchmark --chain=dev --pallet=pallet_balances '--extrinsic=*' --steps=50 --repeat=20 --output=.`
2020-12-08 15:31:37  💸 new validator set of size 1 has been elected via ElectionCompute::OnChain for era 0    

====================

Version: 0.8.27-20f3c411-x86_64-linux-gnu

   0: sp_panic_handler::set::{{closure}}
   1: std::panicking::rust_panic_with_hook
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/std/src/panicking.rs:581
   2: std::panicking::begin_panic_handler::{{closure}}
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/std/src/panicking.rs:484
   3: std::sys_common::backtrace::__rust_end_short_backtrace
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/std/src/sys_common/backtrace.rs:153
   4: rust_begin_unwind
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/std/src/panicking.rs:483
   5: core::panicking::panic_fmt
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/core/src/panicking.rs:85
   6: core::panicking::panic
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/core/src/panicking.rs:50
   7: frame_benchmarking_cli::writer::write_results
   8: frame_benchmarking_cli::command::<impl frame_benchmarking_cli::BenchmarkCmd>::run
   9: polkadot_cli::command::run
  10: polkadot::main
  11: std::sys_common::backtrace::__rust_begin_short_backtrace
  12: std::rt::lang_start::{{closure}}
  13: core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/core/src/ops/function.rs:259
      std::panicking::try::do_call
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/std/src/panicking.rs:381
      std::panicking::try
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/std/src/panicking.rs:345
      std::panic::catch_unwind
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/std/src/panic.rs:382
      std::rt::lang_start_internal
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/std/src/rt.rs:51
  14: main
  15: __libc_start_main
  16: _start


Thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', /home/martin/.cargo/git/checkouts/substrate-7e08433d4c370a21/f9d9382/utils/frame/benchmarking-cli/src/writer.rs:152

This is a bug. Please report it at:

        https://github.com/paritytech/polkadot/issues/new

Notes

The issue appears to affect the following pallets:

• pallet_balances
• pallet_democracy
• pallet_indices
• pallet_session
• pallet_staking
• pallet_timestamp
• pallet_treasury

This bug only occurs when attempting to write the output using --output=some_location.

This bug does not occur when attempting to run the benchmarking subcommand on the pallets inside the Substrate repository. Although invocation is slightly different, I believe we are still calling write_results. For this reason I did not file in the Substrate repo:

% ./substrate benchmark --chain=dev --pallet=pallet_balances --extrinsic=transfer --execution=wasm --wasm-execution=compiled --output=./test
2020-12-08 16:39:07  💸 new validator set of size 1 has been elected via ElectionCompute::OnChain for era 0    
Pallet: "pallet_balances", Extrinsic: "transfer", Lowest values: [], Highest values: [], Steps: [], Repeat: 1
Median Slopes Analysis
========
-- Extrinsic Time --

Model:
Time ~=    116.1
              µs

Reads = 1
Writes = 1
Min Squares Analysis
========
-- Extrinsic Time --

Model:
Time ~=    116.1
              µs

Reads = 1
Writes = 1
x@beer [16:39:08] [~/parity/code/polkadot] [mp-update-weights *]
💩 % cat test

//! Autogenerated weights for pallet_balances
//!
//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 2.0.0
//! DATE: 2020-12-08, STEPS: [], REPEAT: 1, LOW RANGE: [], HIGH RANGE: []
//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 128

// Executed Command:
// ./substrate
// benchmark
// --chain=dev
// --pallet=pallet_balances
// --extrinsic=transfer
// --execution=wasm
// --wasm-execution=compiled
// --output=./test


#![allow(unused_parens)]
#![allow(unused_imports)]

use frame_support::{traits::Get, weights::Weight};
use sp_std::marker::PhantomData;

/// Weight functions for pallet_balances.
pub struct WeightInfo<T>(PhantomData<T>);
impl<T: frame_system::Config> pallet_balances::WeightInfo for WeightInfo<T> {
	fn transfer() -> Weight {
		(116_116_000 as Weight)
			.saturating_add(T::DbWeight::get().reads(1 as Weight))
			.saturating_add(T::DbWeight::get().writes(1 as Weight))
	}
}

Impact

This is currently stopping me from proceeding with the release of Polkadot, as part of the release process entails updating weights.

[s3krit]

After some git bisecting I believe this is a result of paritytech/substrate@3927809 - which means I should probably move this to the Substrate repo

[bkchr]

Cc @shawntabrizi