From 1760815bc71f099bbcc0159e7af51316405b3823 Mon Sep 17 00:00:00 2001
From: emostov <32168567+emostov@users.noreply.github.com>
Date: Thu, 23 Sep 2021 10:46:10 -0700
Subject: [PATCH] make insert_at_unchecked infallible

---
 frame/bags-list/src/list/mod.rs   | 23 +++++++++--------------
 frame/bags-list/src/list/tests.rs |  4 ++--
 2 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs
index 932ef24084e9c..e9e8455c2385b 100644
--- a/frame/bags-list/src/list/mod.rs
+++ b/frame/bags-list/src/list/mod.rs
@@ -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)
 		});
@@ -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() {
@@ -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())
@@ -461,8 +458,6 @@ impl<T: Config> List<T> {
 		// write the updated nodes to storage.
 		at.put();
 		node.put();
-
-		Ok(())
 	}
 
 	/// Sanity check the list.
diff --git a/frame/bags-list/src/list/tests.rs b/frame/bags-list/src/list/tests.rs
index 5eff1eebce601..ceea5b4da2748 100644
--- a/frame/bags-list/src/list/tests.rs
+++ b/frame/bags-list/src/list/tests.rs
@@ -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 };