Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Use BTreeSet as the internal type of ParachainsCache #6795

Merged
merged 3 commits into from
Feb 27, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 8 additions & 11 deletions runtime/parachains/src/paras/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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>>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this 100% have the same SCALE encoding format as the Vec? If not, this will break things, badly.

_config: PhantomData<T>,
}

Expand All @@ -2136,32 +2136,29 @@ impl<T: Config> ParachainsCache<T> {
Self { parachains: None, _config: PhantomData }
}

fn ensure_initialized(&mut self) -> &mut Vec<ParaId> {
self.parachains.get_or_insert_with(|| Parachains::<T>::get())
fn ensure_initialized(&mut self) -> &mut BTreeSet<ParaId> {
self.parachains
.get_or_insert_with(|| Parachains::<T>::get().iter().cloned().collect())
antonva marked this conversation as resolved.
Show resolved Hide resolved
}

/// 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(i, id);
}
parachains.insert(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(i);
}
parachains.remove(&id);
}
}

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.iter().cloned().collect::<Vec<ParaId>>());
}
}
}
74 changes: 74 additions & 0 deletions runtime/parachains/src/paras/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
});
}