Add extrinsic to improve position in a bag of bags-list

frame/bags-list/src/benchmarks.rs

@@ -134,6 +134,44 @@ frame_benchmarking::benchmarks! {
 			]
 		);
 	}
+
+	put_in_front_of {
+		// The most expensive case for `put_in_front_of`:
+		//
+		// - both heavier's `prev` and `next` are nodes that will need to be read and written.
+		// - `lighter` is the bag's `head`, so the bag will need to be read and written.
+
+		let bag_thresh = T::BagThresholds::get()[0];
+
+		// insert the nodes in order
+		let lighter: T::AccountId = account("lighter", 0, 0);
+		assert_ok!(List::<T>::insert(lighter.clone(), bag_thresh));
+
+		let heavier_prev: T::AccountId = account("heavier_prev", 0, 0);
+		assert_ok!(List::<T>::insert(heavier_prev.clone(), bag_thresh));
+
+		let heavier: T::AccountId = account("heavier", 0, 0);
+		assert_ok!(List::<T>::insert(heavier.clone(), bag_thresh));
+
+		let heavier_next: T::AccountId = account("heavier_next", 0, 0);
+		assert_ok!(List::<T>::insert(heavier_next.clone(), bag_thresh));
+
+		T::VoteWeightProvider::set_vote_weight_of(&lighter, bag_thresh - 1);
+		T::VoteWeightProvider::set_vote_weight_of(&heavier, bag_thresh);
+
+		assert_eq!(
+			List::<T>::iter().map(|n| n.id().clone()).collect::<Vec<_>>(),
+			vec![lighter.clone(), heavier_prev.clone(), heavier.clone(), heavier_next.clone()]
+		);
+
+		let caller = whitelisted_caller();
+	}: _(SystemOrigin::Signed(caller), lighter.clone(), heavier.clone())
+	verify {
+		assert_eq!(
+			List::<T>::iter().map(|n| n.id().clone()).collect::<Vec<_>>(),
+			vec![heavier, lighter, heavier_prev, heavier_next]
+		)
+	}
 }
 
 use frame_benchmarking::impl_benchmark_test_suite;

frame/bags-list/src/lib.rs

@@ -178,6 +178,21 @@ pub mod pallet {
 		Rebagged(T::AccountId, VoteWeight, VoteWeight),
 	}
 
+	#[pallet::error]
+	#[cfg_attr(test, derive(PartialEq))]
+	pub enum Error<T> {
+		/// Attempted to place node in front of a node in another bag.
+		NotInSameBag,
+		/// Id not found in list.
+		IdNotFound,
+		/// An Id does not have a greater vote weight than another Id.
+		NotHeavier,
+		/// Heavier weight Id is already in a higher position than a lesser weight Id.
+		AlreadyHigherPosition,
+		/// Bag could not be found. This is a system logic error that should never happen.
+		BagNotFound,
+	}
+
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {
 		/// Declare that some `dislocated` account has, through rewards or penalties, sufficiently
@@ -195,6 +210,26 @@ pub mod pallet {
 			let _ = Pallet::<T>::do_rebag(&dislocated, current_weight);
 			Ok(())
 		}
+
+		/// Move `heavier` directly in front of `lighter`.
+		///
+		/// Only works if
+		/// - both nodes are within the same bag,
+		/// - and `heavier` has a greater `VoteWeight` than `lighter`.
+		#[pallet::weight(T::WeightInfo::put_in_front_of())]
+		pub fn put_in_front_of(
+			origin: OriginFor<T>,
+			heavier: T::AccountId,
+			lighter: T::AccountId,
+		) -> DispatchResult {
+			ensure_signed(origin)?;
+			List::<T>::put_in_front_of(
+				&heavier,
+				&lighter,
+				Box::new(T::VoteWeightProvider::vote_weight),
+			)
+			.map_err(Into::into)
+		}
 	}
 
 	#[pallet::hooks]

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

@@ -384,6 +384,77 @@ impl<T: Config> List<T> {
 		})
 	}
 
+	/// Put `heavier_id` to the position directly in front of `lighter_id`. Both ids must be in
+	/// same bag  and the `weight_of` `lighter_id` must be less than that of `heavier_id`
+	pub(crate) fn put_in_front_of(
+		lighter_id: &T::AccountId,
+		heavier_id: &T::AccountId,
+		weight_of: Box<dyn Fn(&T::AccountId) -> VoteWeight>,
+	) -> Result<(), crate::pallet::Error<T>> {
+		use crate::pallet;
+		use frame_support::ensure;
+
+		let lighter_node = Node::<T>::get(&lighter_id).ok_or(pallet::Error::IdNotFound)?;
+		let mut heavier_node = Node::<T>::get(&heavier_id).ok_or(pallet::Error::IdNotFound)?;
+
+		ensure!(lighter_node.bag_upper == heavier_node.bag_upper, pallet::Error::NotInSameBag);
+		ensure!(
+			heavier_node.next.as_ref() != Some(lighter_id),
+			pallet::Error::AlreadyHigherPosition
+		);
+
+		// this is the most expensive check, so we do it last
+		ensure!(weight_of(&heavier_id) > weight_of(&lighter_id), pallet::Error::NotHeavier);
+
+		// we need to write the bag to storage if its head and/or tail is updated
+		if lighter_node.is_terminal() || heavier_node.is_terminal() {
+			let mut bag = Bag::<T>::get(lighter_node.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
+			})?;
+			debug_assert!(bag.iter().count() > 1);
+
+			if bag.head.as_ref() == Some(lighter_id) {
+				debug_assert!(lighter_node.next.is_some(), "lighter node must have next if head");
+				bag.head = Some(heavier_id.clone());
+			}
+
+			// re-assign bag tail if lighter is the tail
+			if bag.tail.as_ref() == Some(heavier_id) {
+				debug_assert!(heavier_node.prev.is_some(), "heavier node must have prev if tail");
+				bag.tail = heavier_node.prev.clone();
+			}
+
+			// within this block we know the bag must have been updated, so we update storage
+			bag.put()
+		}
+
+		// cut heavier out of the list, updating its neighbors
+		heavier_node.excise();
+
+		// re-fetch `lighter_node` from storage since it may have been updated when heavier node was
+		// excised
+		let mut lighter_node = Node::<T>::get(&lighter_id).ok_or(pallet::Error::IdNotFound)?; // TODO this should never fail, but if does could be bad news
+
+		// connect `heavier_node` to its new `prev`
+		if let Some(mut prev) = lighter_node.prev() {
+			prev.next = Some(heavier_id.clone());
+			prev.put()
+		}
+		heavier_node.prev = lighter_node.prev;
+
+		// connect `heavier_node` and `lighter`
+		heavier_node.next = Some(lighter_id.clone());
+		lighter_node.prev = Some(heavier_id.clone());
+
+		// write the updated nodes to storage
+		lighter_node.put();
+		heavier_node.put();
+
+		Ok(())
+	}
+
 	/// Sanity check the list.
 	///
 	/// This should be called from the call-site, whenever one of the mutating apis (e.g. `insert`)