Benchmark scenario coverage gaps and one incorrect benchmark call (quota::do_refund)

Context

During review of all benchmarking.rs files in the repository, I found several benchmark design issues that can reduce confidence in worst-case weight calibration for some pallets.

This issue tracks the concrete findings and proposed follow-up.

Findings

1) quota::do_refund benchmark calls the wrong function (High)

In pallets/quota/src/benchmarking.rs, benchmark do_refund executes Pallet::<T>::try_refund(refund) instead of Pallet::<T>::do_refund(...).

  • Benchmark location: pallets/quota/src/benchmarking.rs (do_refund, line with try_refund)
  • Actual function to benchmark: pallets/quota/src/lib.rs (do_refund)
  • Weight interface has distinct entries for both:
    • pallets/quota/src/weights.rs::try_refund()
    • pallets/quota/src/weights.rs::do_refund()

Risk: WeightInfo::do_refund() may be calibrated from an incorrect execution path.

2) authority-members benchmarks likely under-cover high-cardinality vector cases (Medium)

pallets/authority-members/src/benchmarking.rs uses tiny fixed setup (new_test_ext(2)) and often benchmarks against the first element only, while runtime logic performs sorted vector binary_search/insert/remove over authority sets.

Relevant runtime operations:

  • insert_in, insert_out, remove_in, remove_online, remove_out
  • all in pallets/authority-members/src/lib.rs

Configured upper bound in runtimes:

  • MaxAuthorities = 32 (runtime/g1/src/parameters.rs, runtime/gtest/src/parameters.rs)

Risk: under-representation of worst-case mutation/search positions and larger vector costs.

3) smith-members benchmarks do not stress MaxByIssuer-driven worst cases (Medium)

pallets/smith-members/src/benchmarking.rs uses small fixed graph/state and does not sweep states approaching T::MaxByIssuer.

Runtime paths with list operations and bound checks:

  • issued_certs < T::MaxByIssuer
  • push/sort on issued_certs
  • binary_search/remove during exclusion paths

Relevant files:

  • pallets/smith-members/src/benchmarking.rs
  • pallets/smith-members/src/lib.rs

Runtime parameter divergence makes this more visible:

  • g1: SmithMaxByIssuer = 12
  • gtest: SmithMaxByIssuer = 100

Risk: potential underweight for higher-bound runtime configurations.

4) Off-by-one setup mismatch in identity::prune_item_identities_names benchmark (Low)

In pallets/identity/src/benchmarking.rs, benchmark parameter i creates i identities, but the names list passed to prune_item_identities_names is built with for k in 1..i, i.e. i - 1 elements.

Risk: mild calibration mismatch between declared benchmark component and actual exercised count.

Proposed Actions

  1. Fix quota::do_refund benchmark to call do_refund directly and regenerate affected weights.
  2. Extend authority-members benchmarks to exercise near-MaxAuthorities states and adverse insertion/removal positions.
  3. Extend smith-members benchmarks with scenarios approaching MaxByIssuer bounds; regenerate weights (at least for runtimes with higher bounds).
  4. Fix identity::prune_item_identities_names setup to align exercised item count with the benchmark parameter.
  5. Re-run benchmark generation and compare weight deltas, then document any intentional conservatism.

References

  • pallets/quota/src/benchmarking.rs
  • pallets/quota/src/lib.rs
  • pallets/quota/src/weights.rs
  • pallets/authority-members/src/benchmarking.rs
  • pallets/authority-members/src/lib.rs
  • pallets/smith-members/src/benchmarking.rs
  • pallets/smith-members/src/lib.rs
  • pallets/identity/src/benchmarking.rs
  • runtime/g1/src/parameters.rs
  • runtime/gtest/src/parameters.rs