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

Commit

Permalink
make insert_at_unchecked infallible
Browse files Browse the repository at this point in the history
  • Loading branch information
emostov committed Sep 23, 2021
1 parent 21aef98 commit 1760815
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 16 deletions.
23 changes: 9 additions & 14 deletions frame/bags-list/src/list/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -409,11 +409,10 @@ impl<T: Config> List<T> {
// re-fetch `lighter_node` from storage since it may have been updated when `heavier_node`
// was removed.
let lighter_node = Node::<T>::get(&lighter_id).ok_or(pallet::Error::IdNotFound)?;
Self::insert_at_unchecked(lighter_node, heavier_node);

Self::insert_at(lighter_node, heavier_node)?;

// since `insert_at` was successful, increment the counter, which got decremented by
// `Self::remove`.
// Increment the counter, which got decremented by `Self::remove`, since we just re-inserted
// `heavier_node`.
crate::CounterForListNodes::<T>::mutate(|prev_count| {
*prev_count = prev_count.saturating_add(1)
});
Expand All @@ -422,9 +421,10 @@ impl<T: Config> List<T> {
}

/// Insert `node` directly in front of `at`.
fn insert_at(mut at: Node<T>, mut node: Node<T>) -> Result<(), crate::pallet::Error<T>> {
use crate::pallet;

///
/// WARNING: we do not check that `node` or `at` are valid. This will panic if `at.bag_upper` is
/// not a bag that already exists in storage.
fn insert_at_unchecked(mut at: Node<T>, mut node: Node<T>) {
// connect `heavier_node` to its new `prev`.
node.prev = at.prev.clone();
if let Some(mut prev) = at.prev() {
Expand All @@ -441,11 +441,8 @@ impl<T: Config> List<T> {
// since `node` is always in front of `at` we know that 1) there is always at least 2
// nodes in the bag, and 2) only `node` could be the head and only `at` could be the
// tail.
let mut bag = Bag::<T>::get(at.bag_upper).ok_or_else(|| {
debug_assert!(false, "bag that should exist cannot be found");
crate::log!(warn, "bag that should exist cannot be found");
pallet::Error::BagNotFound
})?;
let mut bag =
Bag::<T>::get(at.bag_upper).expect("given nodes must always have a valid. qed.");

if node.prev == None {
bag.head = Some(node.id().clone())
Expand All @@ -461,8 +458,6 @@ impl<T: Config> List<T> {
// write the updated nodes to storage.
at.put();
node.put();

Ok(())
}

/// Sanity check the list.
Expand Down
4 changes: 2 additions & 2 deletions frame/bags-list/src/list/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,8 +368,8 @@ mod list {
}

#[test]
#[cfg_attr(debug_assertions, should_panic = "bag that should exist cannot be found")]
fn put_in_front_of_exits_early_if_bag_not_found() {
#[should_panic = "given nodes must always have a valid. qed."]
fn put_in_front_of_panics_if_bag_not_found() {
ExtBuilder::default().build_and_execute_no_post_check(|| {
let node_10_no_bag = Node::<Runtime> { id: 10, prev: None, next: None, bag_upper: 15 };
let node_11_no_bag = Node::<Runtime> { id: 11, prev: None, next: None, bag_upper: 15 };
Expand Down

0 comments on commit 1760815

Please sign in to comment.