Use BTreeSet as the internal type of ParachainsCache (#6795)

* Add tests for ParachainsCache

* Use BTreeSet for ParachainsCache internals

* Use `into_iter()` instead of cloneing storage vals

polkadot/runtime/parachains/src/paras/mod.rs

@@ -122,7 +122,7 @@ use sp_runtime::{
 	traits::{AppVerify, One, Saturating},
 	DispatchResult, SaturatedConversion,
 };
-use sp_std::{cmp, mem, prelude::*};
+use sp_std::{cmp, collections::btree_set::BTreeSet, mem, prelude::*};
 
 #[cfg(feature = "std")]
 use serde::{Deserialize, Serialize};
@@ -2127,7 +2127,7 @@ impl<T: Config> Pallet<T> {
 /// or removing parachains in bulk.
 pub(crate) struct ParachainsCache<T: Config> {
 	// `None` here means the parachains list has not been accessed yet, nevermind modified.
-	parachains: Option<Vec<ParaId>>,
+	parachains: Option<BTreeSet<ParaId>>,
 	_config: PhantomData<T>,
 }
 
@@ -2136,32 +2136,29 @@ impl<T: Config> ParachainsCache<T> {
 		Self { parachains: None, _config: PhantomData }
 	}
 
-	fn ensure_initialized(&mut self) -> &mut Vec<ParaId> {
+	fn ensure_initialized(&mut self) -> &mut BTreeSet<ParaId> {
-		self.parachains.get_or_insert_with(|| Parachains::<T>::get())
+		self.parachains
+			.get_or_insert_with(|| Parachains::<T>::get().into_iter().collect())
 	}
 
 	/// Adds the given para id to the list.
 	pub fn add(&mut self, id: ParaId) {
 		let parachains = self.ensure_initialized();
-		if let Err(i) = parachains.binary_search(&id) {
+		parachains.insert(id);
-			parachains.insert(i, id);
-		}
 	}
 
 	/// Removes the given para id from the list of parachains. Does nothing if the id is not in the
 	/// list.
 	pub fn remove(&mut self, id: ParaId) {
 		let parachains = self.ensure_initialized();
-		if let Ok(i) = parachains.binary_search(&id) {
+		parachains.remove(&id);
-			parachains.remove(i);
-		}
 	}
 }
 
 impl<T: Config> Drop for ParachainsCache<T> {
 	fn drop(&mut self) {
 		if let Some(parachains) = self.parachains.take() {
-			Parachains::<T>::put(&parachains);
+			Parachains::<T>::put(parachains.into_iter().collect::<Vec<ParaId>>());
 		}
 	}
 }

polkadot/runtime/parachains/src/paras/tests.rs

@@ -1767,3 +1767,77 @@ fn parakind_encodes_decodes_to_bool_serde() {
 	let ser_false = serde_json::to_string(&false).unwrap();
 	assert_eq!(ser_false, ser_thread);
 }
+
+#[test]
+fn parachains_cache_is_set() {
+	new_test_ext(MockGenesisConfig::default()).execute_with(|| {
+		let a = ParaId::from(111);
+
+		let mut parachains_cache: ParachainsCache<Test> = ParachainsCache::new();
+
+		// Add element twice
+		parachains_cache.add(a);
+		parachains_cache.add(a);
+
+		// Flush cache to storage
+		drop(parachains_cache);
+
+		// In order after addition
+		assert_eq!(<Paras as Store>::Parachains::get(), vec![a]);
+
+		let mut parachains_cache: ParachainsCache<Test> = ParachainsCache::new();
+
+		// Remove element twice
+		parachains_cache.remove(a);
+		parachains_cache.remove(a);
+
+		// Flush cache to storage
+		drop(parachains_cache);
+
+		// In order after removal
+		assert_eq!(<Paras as Store>::Parachains::get(), vec![]);
+
+		let mut parachains_cache: ParachainsCache<Test> = ParachainsCache::new();
+
+		// Remove nonexisting element
+		parachains_cache.remove(a);
+		assert_storage_noop!(drop(parachains_cache));
+		assert_eq!(<Paras as Store>::Parachains::get(), vec![]);
+	});
+}
+
+#[test]
+fn parachains_cache_preserves_order() {
+	new_test_ext(MockGenesisConfig::default()).execute_with(|| {
+		let a = ParaId::from(111);
+		let b = ParaId::from(222);
+		let c = ParaId::from(333);
+		let d = ParaId::from(444);
+
+		let mut parachains_cache: ParachainsCache<Test> = ParachainsCache::new();
+
+		// Add in mixed order
+		parachains_cache.add(b);
+		parachains_cache.add(c);
+		parachains_cache.add(a);
+		parachains_cache.add(d);
+
+		// Flush cache to storage
+		drop(parachains_cache);
+
+		// In order after addition
+		assert_eq!(<Paras as Store>::Parachains::get(), vec![a, b, c, d]);
+
+		let mut parachains_cache: ParachainsCache<Test> = ParachainsCache::new();
+
+		// Remove 2 elements
+		parachains_cache.remove(b);
+		parachains_cache.remove(d);
+
+		// Flush cache to storage
+		drop(parachains_cache);
+
+		// In order after removal
+		assert_eq!(<Paras as Store>::Parachains::get(), vec![a, c]);
+	});
+}