Fix pallet bags list and doc (#10231)

* fix bags list

* improve doc

* doc

* inner doc

* fix test

* Update docs in frame/election-provider-support/src/lib.rs

* fix staking impl

* prepend unsafe to clear and regenerate

* fix test

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

substrate/frame/bags-list/src/benchmarks.rs

@@ -35,7 +35,8 @@ frame_benchmarking::benchmarks! {
 		//   node in the destination in addition to the work we do otherwise. (2 W/R)
 
 		// clear any pre-existing storage.
-		List::<T>::clear(None);
+		// NOTE: safe to call outside block production
+		List::<T>::unsafe_clear();
 
 		// define our origin and destination thresholds.
 		let origin_bag_thresh = T::BagThresholds::get()[0];
@@ -94,7 +95,8 @@ frame_benchmarking::benchmarks! {
 		//   node in the destination in addition to the work we do otherwise. (2 W/R)
 
 		// clear any pre-existing storage.
-		List::<T>::clear(None);
+		// NOTE: safe to call outside block production
+		List::<T>::unsafe_clear();
 
 		// define our origin and destination thresholds.
 		let origin_bag_thresh = T::BagThresholds::get()[0];

substrate/frame/bags-list/src/lib.rs

@@ -26,7 +26,7 @@
 //! the weights of accounts via [`frame_election_provider_support::VoteWeightProvider`].
 //!
 //! This pallet is not configurable at genesis. Whoever uses it should call appropriate functions of
-//! the `SortedListProvider` (e.g. `on_insert`, or `regenerate`) at their genesis.
+//! the `SortedListProvider` (e.g. `on_insert`, or `unsafe_regenerate`) at their genesis.
 //!
 //! # Goals
 //!
@@ -256,11 +256,14 @@ impl<T: Config> SortedListProvider<T::AccountId> for Pallet<T> {
 		List::<T>::remove(id)
 	}
 
-	fn regenerate(
+	fn unsafe_regenerate(
 		all: impl IntoIterator<Item = T::AccountId>,
 		weight_of: Box<dyn Fn(&T::AccountId) -> VoteWeight>,
 	) -> u32 {
-		List::<T>::regenerate(all, weight_of)
+		// NOTE: This call is unsafe for the same reason as SortedListProvider::unsafe_regenerate.
+		// I.e. because it can lead to many storage accesses.
+		// So it is ok to call it as caller must ensure the conditions.
+		List::<T>::unsafe_regenerate(all, weight_of)
 	}
 
 	#[cfg(feature = "std")]
@@ -273,8 +276,11 @@ impl<T: Config> SortedListProvider<T::AccountId> for Pallet<T> {
 		Ok(())
 	}
 
-	fn clear(maybe_count: Option<u32>) -> u32 {
-		List::<T>::clear(maybe_count)
+	fn unsafe_clear() {
+		// NOTE: This call is unsafe for the same reason as SortedListProvider::unsafe_clear.
+		// I.e. because it can lead to many storage accesses.
+		// So it is ok to call it as caller must ensure the conditions.
+		List::<T>::unsafe_clear()
 	}
 
 	#[cfg(feature = "runtime-benchmarks")]

substrate/frame/bags-list/src/list/mod.rs

@@ -76,19 +76,15 @@ pub fn notional_bag_for<T: Config>(weight: VoteWeight) -> VoteWeight {
 pub struct List<T: Config>(PhantomData<T>);
 
 impl<T: Config> List<T> {
-	/// Remove all data associated with the list from storage. Parameter `items` is the number of
-	/// items to clear from the list.
+	/// Remove all data associated with the list from storage.
 	///
 	/// ## WARNING
 	///
-	/// `None` will clear all items and should generally not be used in production as it could lead
-	/// to a very large number of storage accesses.
-	pub(crate) fn clear(maybe_count: Option<u32>) -> u32 {
-		crate::ListBags::<T>::remove_all(maybe_count);
-		let pre = crate::ListNodes::<T>::count();
-		crate::ListNodes::<T>::remove_all(maybe_count);
-		let post = crate::ListNodes::<T>::count();
-		pre.saturating_sub(post)
+	/// this function should generally not be used in production as it could lead to a very large
+	/// number of storage accesses.
+	pub(crate) fn unsafe_clear() {
+		crate::ListBags::<T>::remove_all(None);
+		crate::ListNodes::<T>::remove_all();
 	}
 
 	/// Regenerate all of the data from the given ids.
@@ -100,11 +96,14 @@ impl<T: Config> List<T> {
 	/// pallet using this `List`.
 	///
 	/// Returns the number of ids migrated.
-	pub fn regenerate(
+	pub fn unsafe_regenerate(
 		all: impl IntoIterator<Item = T::AccountId>,
 		weight_of: Box<dyn Fn(&T::AccountId) -> VoteWeight>,
 	) -> u32 {
-		Self::clear(None);
+		// NOTE: This call is unsafe for the same reason as SortedListProvider::unsafe_regenerate.
+		// I.e. because it can lead to many storage accesses.
+		// So it is ok to call it as caller must ensure the conditions.
+		Self::unsafe_clear();
 		Self::insert_many(all, weight_of)
 	}