diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs
index c1e6dad7bf0d4e060530784d645a9dff422c415a..c98bb5cc13f8525cff9c1aeaf3a92a3796ba8401 100644
--- a/substrate/bin/node/runtime/src/lib.rs
+++ b/substrate/bin/node/runtime/src/lib.rs
@@ -1572,7 +1572,7 @@ pub type Executive = frame_executive::Executive<
frame_system::ChainContext<Runtime>,
Runtime,
AllPalletsWithSystem,
- pallet_bags_list::migrations::CheckCounterPrefix<Runtime>,
+ (),
>;
/// MMR helper types.
diff --git a/substrate/frame/bags-list/fuzzer/src/main.rs b/substrate/frame/bags-list/fuzzer/src/main.rs
index d0586f0372ac4d118061f0c615678b71cca6a605..c17fbe0a2f77f4a2d01f73f52a84b99a01706644 100644
--- a/substrate/frame/bags-list/fuzzer/src/main.rs
+++ b/substrate/frame/bags-list/fuzzer/src/main.rs
@@ -70,13 +70,20 @@ fn main() {
},
Action::Update => {
let already_contains = BagsList::contains(&id);
- BagsList::on_update(&id, vote_weight).unwrap();
if already_contains {
+ BagsList::on_update(&id, vote_weight).unwrap();
assert!(BagsList::contains(&id));
+ } else {
+ BagsList::on_update(&id, vote_weight).unwrap_err();
}
},
Action::Remove => {
- BagsList::on_remove(&id).unwrap();
+ let already_contains = BagsList::contains(&id);
+ if already_contains {
+ BagsList::on_remove(&id).unwrap();
+ } else {
+ BagsList::on_remove(&id).unwrap_err();
+ }
assert!(!BagsList::contains(&id));
},
}
diff --git a/substrate/frame/bags-list/src/lib.rs b/substrate/frame/bags-list/src/lib.rs
index 77fe609ad8123d904a30b2370da167aca241bf6f..7eee8fdfa23d8de8a4c83d09dc96ed3c4b99fd8c 100644
--- a/substrate/frame/bags-list/src/lib.rs
+++ b/substrate/frame/bags-list/src/lib.rs
@@ -82,7 +82,10 @@ macro_rules! log {
($level:tt, $patter:expr $(, $values:expr)* $(,)?) => {
log::$level!(
target: crate::LOG_TARGET,
- concat!("[{:?}] 👜", $patter), <frame_system::Pallet<T>>::block_number() $(, $values)*
+ concat!("[{:?}] 👜 [{}]", $patter),
+ <frame_system::Pallet<T>>::block_number(),
+ <crate::Pallet::<T, I> as frame_support::traits::PalletInfoAccess>::name()
+ $(, $values)*
)
};
}
@@ -189,6 +192,8 @@ pub mod pallet {
pub enum Event<T: Config<I>, I: 'static = ()> {
/// Moved an account from one bag to another.
Rebagged { who: T::AccountId, from: T::Score, to: T::Score },
+ /// Updated the score of some account to the given amount.
+ ScoreUpdated { who: T::AccountId, new_score: T::Score },
}
#[pallet::error]
@@ -212,13 +217,16 @@ pub mod pallet {
///
/// Anyone can call this function about any potentially dislocated account.
///
- /// Will never return an error; if `dislocated` does not exist or doesn't need a rebag, then
- /// it is a noop and fees are still collected from `origin`.
+ /// Will always update the stored score of `dislocated` to the correct score, based on
+ /// `ScoreProvider`.
+ ///
+ /// If `dislocated` does not exists, it returns an error.
#[pallet::weight(T::WeightInfo::rebag_non_terminal().max(T::WeightInfo::rebag_terminal()))]
pub fn rebag(origin: OriginFor<T>, dislocated: T::AccountId) -> DispatchResult {
ensure_signed(origin)?;
let current_score = T::ScoreProvider::score(&dislocated);
- let _ = Pallet::<T, I>::do_rebag(&dislocated, current_score);
+ let _ = Pallet::<T, I>::do_rebag(&dislocated, current_score)
+ .map_err::<Error<T, I>, _>(Into::into)?;
Ok(())
}
@@ -265,6 +273,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
if let Some((from, to)) = maybe_movement {
Self::deposit_event(Event::<T, I>::Rebagged { who: account.clone(), from, to });
};
+ Self::deposit_event(Event::<T, I>::ScoreUpdated { who: account.clone(), new_score });
Ok(maybe_movement)
}
@@ -302,6 +311,10 @@ impl<T: Config<I>, I: 'static> SortedListProvider<T::AccountId> for Pallet<T, I>
List::<T, I>::insert(id, score)
}
+ fn get_score(id: &T::AccountId) -> Result<T::Score, ListError> {
+ List::<T, I>::get_score(id)
+ }
+
fn on_update(id: &T::AccountId, new_score: T::Score) -> Result<(), ListError> {
Pallet::<T, I>::do_rebag(id, new_score).map(|_| ())
}
@@ -359,3 +372,22 @@ impl<T: Config<I>, I: 'static> SortedListProvider<T::AccountId> for Pallet<T, I>
}
}
}
+
+impl<T: Config<I>, I: 'static> ScoreProvider<T::AccountId> for Pallet<T, I> {
+ type Score = <Pallet<T, I> as SortedListProvider<T::AccountId>>::Score;
+
+ fn score(id: &T::AccountId) -> T::Score {
+ Node::<T, I>::get(id).map(|node| node.score()).unwrap_or_default()
+ }
+
+ #[cfg(any(feature = "runtime-benchmarks", test))]
+ fn set_score_of(id: &T::AccountId, new_score: T::Score) {
+ ListNodes::<T, I>::mutate(id, |maybe_node| {
+ if let Some(node) = maybe_node.as_mut() {
+ node.set_score(new_score)
+ } else {
+ panic!("trying to mutate {:?} which does not exists", id);
+ }
+ })
+ }
+}
diff --git a/substrate/frame/bags-list/src/list/mod.rs b/substrate/frame/bags-list/src/list/mod.rs
index 2e65c3be25b24fda52dace31c52d8764130d328e..d95c5de8132ea53d3148e72e73695a1ac4938f02 100644
--- a/substrate/frame/bags-list/src/list/mod.rs
+++ b/substrate/frame/bags-list/src/list/mod.rs
@@ -28,6 +28,7 @@ use crate::Config;
use codec::{Decode, Encode, MaxEncodedLen};
use frame_election_provider_support::ScoreProvider;
use frame_support::{
+ ensure,
traits::{Defensive, Get},
DefaultNoBound, PalletError,
};
@@ -38,7 +39,7 @@ use sp_std::{
collections::{btree_map::BTreeMap, btree_set::BTreeSet},
iter,
marker::PhantomData,
- vec::Vec,
+ prelude::*,
};
#[derive(Debug, PartialEq, Eq, Encode, Decode, MaxEncodedLen, TypeInfo, PalletError)]
@@ -227,6 +228,11 @@ impl<T: Config<I>, I: 'static> List<T, I> {
crate::ListNodes::<T, I>::contains_key(id)
}
+ /// Get the score of the given node,
+ pub fn get_score(id: &T::AccountId) -> Result<T::Score, ListError> {
+ Node::<T, I>::get(id).map(|node| node.score()).ok_or(ListError::NodeNotFound)
+ }
+
/// Iterate over all nodes in all bags in the list.
///
/// Full iteration can be expensive; it's recommended to limit the number of items with
@@ -310,7 +316,7 @@ impl<T: Config<I>, I: 'static> List<T, I> {
let bag_score = notional_bag_for::<T, I>(score);
let mut bag = Bag::<T, I>::get_or_make(bag_score);
// unchecked insertion is okay; we just got the correct `notional_bag_for`.
- bag.insert_unchecked(id.clone());
+ bag.insert_unchecked(id.clone(), score);
// new inserts are always the tail, so we must write the bag.
bag.put();
@@ -381,16 +387,18 @@ impl<T: Config<I>, I: 'static> List<T, I> {
/// If the node was in the correct bag, no effect. If the node was in the incorrect bag, they
/// are moved into the correct bag.
///
- /// Returns `Some((old_idx, new_idx))` if the node moved, otherwise `None`.
+ /// Returns `Some((old_idx, new_idx))` if the node moved, otherwise `None`. In both cases, the
+ /// node's score is written to the `score` field. Thus, this is not a noop, even if `None`.
///
/// This operation is somewhat more efficient than simply calling [`self.remove`] followed by
/// [`self.insert`]. However, given large quantities of nodes to move, it may be more efficient
/// to call [`self.remove_many`] followed by [`self.insert_many`].
pub(crate) fn update_position_for(
- node: Node<T, I>,
+ mut node: Node<T, I>,
new_score: T::Score,
) -> Option<(T::Score, T::Score)> {
- node.is_misplaced(new_score).then(move || {
+ node.score = new_score;
+ if node.is_misplaced(new_score) {
let old_bag_upper = node.bag_upper;
if !node.is_terminal() {
@@ -402,12 +410,9 @@ impl<T: Config<I>, I: 'static> List<T, I> {
bag.remove_node_unchecked(&node);
bag.put();
} else {
- crate::log!(
- error,
- "Node {:?} did not have a bag; ListBags is in an inconsistent state",
- node.id,
+ frame_support::defensive!(
+ "Node did not have a bag; BagsList is in an inconsistent state"
);
- debug_assert!(false, "every node must have an extant bag associated with it");
}
// put the node into the appropriate new bag.
@@ -418,8 +423,12 @@ impl<T: Config<I>, I: 'static> List<T, I> {
bag.insert_node_unchecked(node);
bag.put();
- (old_bag_upper, new_bag_upper)
- })
+ Some((old_bag_upper, new_bag_upper))
+ } else {
+ // just write the new score.
+ node.put();
+ None
+ }
}
/// Put `heavier_id` to the position directly in front of `lighter_id`. Both ids must be in the
@@ -428,8 +437,6 @@ impl<T: Config<I>, I: 'static> List<T, I> {
lighter_id: &T::AccountId,
heavier_id: &T::AccountId,
) -> Result<(), ListError> {
- use frame_support::ensure;
-
let lighter_node = Node::<T, I>::get(&lighter_id).ok_or(ListError::NodeNotFound)?;
let heavier_node = Node::<T, I>::get(&heavier_id).ok_or(ListError::NodeNotFound)?;
@@ -510,7 +517,6 @@ impl<T: Config<I>, I: 'static> List<T, I> {
/// all bags and nodes are checked per *any* update to `List`.
#[cfg(feature = "std")]
pub(crate) fn sanity_check() -> Result<(), &'static str> {
- use frame_support::ensure;
let mut seen_in_list = BTreeSet::new();
ensure!(
Self::iter().map(|node| node.id).all(|id| seen_in_list.insert(id)),
@@ -523,7 +529,7 @@ impl<T: Config<I>, I: 'static> List<T, I> {
ensure!(iter_count == stored_count, "iter_count != stored_count");
ensure!(stored_count == nodes_count, "stored_count != nodes_count");
- crate::log!(debug, "count of nodes: {}", stored_count);
+ crate::log!(trace, "count of nodes: {}", stored_count);
let active_bags = {
let thresholds = T::BagThresholds::get().iter().copied();
@@ -544,7 +550,7 @@ impl<T: Config<I>, I: 'static> List<T, I> {
active_bags.clone().fold(0u32, |acc, cur| acc + cur.iter().count() as u32);
ensure!(nodes_count == nodes_in_bags_count, "stored_count != nodes_in_bags_count");
- crate::log!(debug, "count of active bags {}", active_bags.count());
+ crate::log!(trace, "count of active bags {}", active_bags.count());
// check that all nodes are sane. We check the `ListNodes` storage item directly in case we
// have some "stale" nodes that are not in a bag.
@@ -667,7 +673,7 @@ impl<T: Config<I>, I: 'static> Bag<T, I> {
///
/// Storage note: this modifies storage, but only for the nodes. You still need to call
/// `self.put()` after use.
- fn insert_unchecked(&mut self, id: T::AccountId) {
+ fn insert_unchecked(&mut self, id: T::AccountId, score: T::Score) {
// insert_node will overwrite `prev`, `next` and `bag_upper` to the proper values. As long
// as this bag is the correct one, we're good. All calls to this must come after getting the
// correct [`notional_bag_for`].
@@ -676,6 +682,7 @@ impl<T: Config<I>, I: 'static> Bag<T, I> {
prev: None,
next: None,
bag_upper: Zero::zero(),
+ score,
_phantom: PhantomData,
});
}
@@ -784,11 +791,6 @@ impl<T: Config<I>, I: 'static> Bag<T, I> {
Ok(())
}
- #[cfg(not(feature = "std"))]
- fn sanity_check(&self) -> Result<(), &'static str> {
- Ok(())
- }
-
/// Iterate over the nodes in this bag (public for tests).
#[cfg(feature = "std")]
#[allow(dead_code)]
@@ -809,12 +811,13 @@ impl<T: Config<I>, I: 'static> Bag<T, I> {
#[scale_info(skip_type_params(T, I))]
#[cfg_attr(feature = "std", derive(frame_support::DebugNoBound, Clone, PartialEq))]
pub struct Node<T: Config<I>, I: 'static = ()> {
- id: T::AccountId,
- prev: Option<T::AccountId>,
- next: Option<T::AccountId>,
- bag_upper: T::Score,
+ pub(crate) id: T::AccountId,
+ pub(crate) prev: Option<T::AccountId>,
+ pub(crate) next: Option<T::AccountId>,
+ pub(crate) bag_upper: T::Score,
+ pub(crate) score: T::Score,
#[codec(skip)]
- _phantom: PhantomData<I>,
+ pub(crate) _phantom: PhantomData<I>,
}
impl<T: Config<I>, I: 'static> Node<T, I> {
@@ -877,6 +880,11 @@ impl<T: Config<I>, I: 'static> Node<T, I> {
&self.id
}
+ /// Get the current vote weight of the node.
+ pub(crate) fn score(&self) -> T::Score {
+ self.score
+ }
+
/// Get the underlying voter (public fo tests).
#[cfg(feature = "std")]
#[allow(dead_code)]
@@ -884,6 +892,11 @@ impl<T: Config<I>, I: 'static> Node<T, I> {
&self.id
}
+ #[cfg(any(feature = "runtime-benchmarks", test))]
+ pub fn set_score(&mut self, s: T::Score) {
+ self.score = s
+ }
+
/// The bag this nodes belongs to (public for benchmarks).
#[cfg(feature = "runtime-benchmarks")]
#[allow(dead_code)]
diff --git a/substrate/frame/bags-list/src/list/tests.rs b/substrate/frame/bags-list/src/list/tests.rs
index 40a174b35a5d3224a8459c0f4a09d73f368b2bd6..9bdd54289fd8856bd3828f64248f608f2b2ce9b7 100644
--- a/substrate/frame/bags-list/src/list/tests.rs
+++ b/substrate/frame/bags-list/src/list/tests.rs
@@ -21,20 +21,20 @@ use crate::{
ListBags, ListNodes,
};
use frame_election_provider_support::{SortedListProvider, VoteWeight};
-use frame_support::{assert_noop, assert_ok, assert_storage_noop};
+use frame_support::{assert_ok, assert_storage_noop};
+
+fn node(
+ id: AccountId,
+ prev: Option<AccountId>,
+ next: Option<AccountId>,
+ bag_upper: VoteWeight,
+) -> Node<Runtime> {
+ Node::<Runtime> { id, prev, next, bag_upper, score: bag_upper, _phantom: PhantomData }
+}
#[test]
fn basic_setup_works() {
ExtBuilder::default().build_and_execute(|| {
- // syntactic sugar to create a raw node
- let node = |id, prev, next, bag_upper| Node::<Runtime> {
- id,
- prev,
- next,
- bag_upper,
- _phantom: PhantomData,
- };
-
assert_eq!(ListNodes::<Runtime>::count(), 4);
assert_eq!(ListNodes::<Runtime>::iter().count(), 4);
assert_eq!(ListBags::<Runtime>::iter().count(), 2);
@@ -83,10 +83,10 @@ fn notional_bag_for_works() {
let max_explicit_threshold = *<Runtime as Config>::BagThresholds::get().last().unwrap();
assert_eq!(max_explicit_threshold, 10_000);
- // if the max explicit threshold is less than T::Value::max_value(),
+ // if the max explicit threshold is less than T::Score::max_value(),
assert!(VoteWeight::MAX > max_explicit_threshold);
- // then anything above it will belong to the T::Value::max_value() bag.
+ // then anything above it will belong to the T::Score::max_value() bag.
assert_eq!(notional_bag_for::<Runtime, _>(max_explicit_threshold), max_explicit_threshold);
assert_eq!(notional_bag_for::<Runtime, _>(max_explicit_threshold + 1), VoteWeight::MAX);
}
@@ -154,6 +154,8 @@ fn migrate_works() {
}
mod list {
+ use frame_support::assert_noop;
+
use super::*;
#[test]
@@ -175,7 +177,7 @@ mod list {
]
);
- // when adding an id that has a higher weight than pre-existing ids in the bag
+ // when adding an id that has a higher score than pre-existing ids in the bag
assert_ok!(List::<Runtime>::insert(7, 10));
// then
@@ -246,10 +248,7 @@ mod list {
assert!(get_list_as_ids().contains(&3));
// then
- assert_storage_noop!(assert_eq!(
- List::<Runtime>::insert(3, 20).unwrap_err(),
- ListError::Duplicate
- ));
+ assert_noop!(List::<Runtime>::insert(3, 20), ListError::Duplicate);
});
}
@@ -317,37 +316,36 @@ mod list {
ExtBuilder::default().build_and_execute(|| {
// given a correctly placed account 1 at bag 10.
let node = Node::<Runtime>::get(&1).unwrap();
+ assert_eq!(node.score, 10);
assert!(!node.is_misplaced(10));
- // .. it is invalid with weight 20
+ // .. it is invalid with score 20
assert!(node.is_misplaced(20));
// move it to bag 20.
- assert_eq!(List::<Runtime>::update_position_for(node, 20), Some((10, 20)));
+ assert_eq!(List::<Runtime>::update_position_for(node.clone(), 20), Some((10, 20)));
+ assert_eq!(Node::<Runtime>::get(&1).unwrap().score, 20);
assert_eq!(List::<Runtime>::get_bags(), vec![(20, vec![1]), (1_000, vec![2, 3, 4])]);
- // get the new updated node; try and update the position with no change in weight.
+ // get the new updated node; try and update the position with no change in score.
let node = Node::<Runtime>::get(&1).unwrap();
assert_storage_noop!(assert_eq!(
List::<Runtime>::update_position_for(node.clone(), 20),
None
));
- // then move it to bag 1_000 by giving it weight 500.
+ // then move it to bag 1_000 by giving it score 500.
assert_eq!(List::<Runtime>::update_position_for(node.clone(), 500), Some((20, 1_000)));
+ assert_eq!(Node::<Runtime>::get(&1).unwrap().score, 500);
assert_eq!(List::<Runtime>::get_bags(), vec![(1_000, vec![2, 3, 4, 1])]);
// moving within that bag again is a noop
let node = Node::<Runtime>::get(&1).unwrap();
- assert_storage_noop!(assert_eq!(
- List::<Runtime>::update_position_for(node.clone(), 750),
- None,
- ));
- assert_storage_noop!(assert_eq!(
- List::<Runtime>::update_position_for(node, 1_000),
- None,
- ));
+ assert_eq!(List::<Runtime>::update_position_for(node.clone(), 750), None);
+ assert_eq!(Node::<Runtime>::get(&1).unwrap().score, 750);
+ assert_eq!(List::<Runtime>::update_position_for(node.clone(), 1_000), None,);
+ assert_eq!(Node::<Runtime>::get(&1).unwrap().score, 1_000);
});
}
@@ -359,7 +357,7 @@ mod list {
// make sure there are no duplicates.
ExtBuilder::default().build_and_execute_no_post_check(|| {
- Bag::<Runtime>::get(10).unwrap().insert_unchecked(2);
+ Bag::<Runtime>::get(10).unwrap().insert_unchecked(2, 10);
assert_eq!(List::<Runtime>::sanity_check(), Err("duplicate identified"));
});
@@ -397,6 +395,7 @@ mod list {
prev: None,
next: None,
bag_upper: 15,
+ score: 15,
_phantom: PhantomData,
};
let node_11_no_bag = Node::<Runtime> {
@@ -404,6 +403,7 @@ mod list {
prev: None,
next: None,
bag_upper: 15,
+ score: 15,
_phantom: PhantomData,
};
@@ -435,6 +435,7 @@ mod list {
prev: Some(1),
next: Some(2),
bag_upper: 1_000,
+ score: 1_000,
_phantom: PhantomData,
};
assert!(!crate::ListNodes::<Runtime>::contains_key(42));
@@ -464,6 +465,7 @@ mod list {
prev: Some(4),
next: None,
bag_upper: 1_000,
+ score: 1_000,
_phantom: PhantomData,
};
assert!(!crate::ListNodes::<Runtime>::contains_key(42));
@@ -493,6 +495,7 @@ mod list {
prev: None,
next: Some(2),
bag_upper: 1_000,
+ score: 1_000,
_phantom: PhantomData,
};
assert!(!crate::ListNodes::<Runtime>::contains_key(42));
@@ -522,6 +525,7 @@ mod list {
prev: Some(42),
next: Some(42),
bag_upper: 1_000,
+ score: 1_000,
_phantom: PhantomData,
};
assert!(!crate::ListNodes::<Runtime>::contains_key(42));
@@ -586,6 +590,7 @@ mod bags {
prev: None,
next: None,
bag_upper,
+ score: bag_upper,
_phantom: PhantomData,
};
@@ -596,7 +601,14 @@ mod bags {
assert_eq!(
ListNodes::<Runtime>::get(&42).unwrap(),
- Node { bag_upper: 10, prev: Some(1), next: None, id: 42, _phantom: PhantomData }
+ Node {
+ bag_upper: 10,
+ score: 5,
+ prev: Some(1),
+ next: None,
+ id: 42,
+ _phantom: PhantomData
+ }
);
});
}
@@ -609,6 +621,7 @@ mod bags {
prev: None,
next: None,
bag_upper,
+ score: bag_upper,
_phantom: PhantomData,
};
@@ -636,6 +649,7 @@ mod bags {
prev: Some(21),
next: Some(101),
bag_upper: 20,
+ score: 20,
_phantom: PhantomData,
};
bag_20.insert_node_unchecked(node_61);
@@ -649,6 +663,7 @@ mod bags {
prev: Some(62),
next: None,
bag_upper: 20,
+ score: 20,
_phantom: PhantomData,
}
);
@@ -665,13 +680,6 @@ mod bags {
// Document improper ways `insert_node` may be getting used.
#[test]
fn insert_node_bad_paths_documented() {
- let node = |id, prev, next, bag_upper| Node::<Runtime> {
- id,
- prev,
- next,
- bag_upper,
- _phantom: PhantomData,
- };
ExtBuilder::default().build_and_execute_no_post_check(|| {
// when inserting a node with both prev & next pointing at an account in an incorrect
// bag.
@@ -684,7 +692,14 @@ mod bags {
// and when the node is re-fetched all the info is correct
assert_eq!(
Node::<Runtime>::get(&42).unwrap(),
- node(42, Some(4), None, bag_1000.bag_upper)
+ Node::<Runtime> {
+ id: 42,
+ prev: Some(4),
+ next: None,
+ bag_upper: bag_1000.bag_upper,
+ score: 500,
+ _phantom: PhantomData
+ }
);
});
@@ -720,7 +735,14 @@ mod bags {
// and the re-fetched node has bad pointers
assert_eq!(
Node::<Runtime>::get(&2).unwrap(),
- node(2, Some(4), None, bag_1000.bag_upper)
+ Node::<Runtime> {
+ id: 2,
+ prev: Some(4),
+ next: None,
+ bag_upper: bag_1000.bag_upper,
+ score: 0,
+ _phantom: PhantomData
+ },
);
// ^^^ despite being the bags head, it has a prev
@@ -739,14 +761,6 @@ mod bags {
)]
fn insert_node_duplicate_tail_panics_with_debug_assert() {
ExtBuilder::default().build_and_execute(|| {
- let node = |id, prev, next, bag_upper| Node::<Runtime> {
- id,
- prev,
- next,
- bag_upper,
- _phantom: PhantomData,
- };
-
// given
assert_eq!(List::<Runtime>::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])],);
let mut bag_1000 = Bag::<Runtime>::get(1_000).unwrap();
@@ -877,6 +891,7 @@ mod bags {
prev: None,
next: Some(3),
bag_upper: 10, // should be 1_000
+ score: 10,
_phantom: PhantomData,
};
let mut bag_1000 = Bag::<Runtime>::get(1_000).unwrap();
diff --git a/substrate/frame/bags-list/src/migrations.rs b/substrate/frame/bags-list/src/migrations.rs
index f8041327f10bee5e497b71626d7908d0e80fb610..a77beb23bd66779c0c21b699d0e726251d4759b7 100644
--- a/substrate/frame/bags-list/src/migrations.rs
+++ b/substrate/frame/bags-list/src/migrations.rs
@@ -17,34 +17,125 @@
//! The migrations of this pallet.
+use codec::{Decode, Encode};
+use core::marker::PhantomData;
+use frame_election_provider_support::ScoreProvider;
use frame_support::traits::OnRuntimeUpgrade;
+use sp_runtime::traits::Zero;
+
+#[cfg(feature = "try-runtime")]
+use frame_support::ensure;
/// A struct that does not migration, but only checks that the counter prefix exists and is correct.
-pub struct CheckCounterPrefix<T: crate::Config>(sp_std::marker::PhantomData<T>);
-impl<T: crate::Config> OnRuntimeUpgrade for CheckCounterPrefix<T> {
+pub struct CheckCounterPrefix<T: crate::Config<I>, I: 'static>(sp_std::marker::PhantomData<(T, I)>);
+impl<T: crate::Config<I>, I: 'static> OnRuntimeUpgrade for CheckCounterPrefix<T, I> {
fn on_runtime_upgrade() -> frame_support::weights::Weight {
- 0
+ frame_support::weights::Weight::zero()
}
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<(), &'static str> {
- use frame_support::ensure;
// The old explicit storage item.
#[frame_support::storage_alias]
- type CounterForListNodes<T: crate::Config> = StorageValue<crate::Pallet<T>, u32>;
+ type CounterForListNodes<T: crate::Config<I>, I: 'static> =
+ StorageValue<crate::Pallet<T, I>, u32>;
// ensure that a value exists in the counter struct.
ensure!(
- crate::ListNodes::<T>::count() == CounterForListNodes::<T>::get().unwrap(),
+ crate::ListNodes::<T, I>::count() == CounterForListNodes::<T, I>::get().unwrap(),
"wrong list node counter"
);
crate::log!(
info,
"checked bags-list prefix to be correct and have {} nodes",
- crate::ListNodes::<T>::count()
+ crate::ListNodes::<T, I>::count()
);
Ok(())
}
}
+
+mod old {
+ use super::*;
+ use frame_support::pallet_prelude::*;
+
+ #[derive(Encode, Decode)]
+ pub struct PreScoreNode<T: crate::Config<I>, I: 'static = ()> {
+ pub id: T::AccountId,
+ pub prev: Option<T::AccountId>,
+ pub next: Option<T::AccountId>,
+ pub bag_upper: T::Score,
+ #[codec(skip)]
+ pub _phantom: PhantomData<I>,
+ }
+
+ #[frame_support::storage_alias]
+ pub type ListNodes<T: crate::Config<I>, I: 'static> = StorageMap<
+ crate::Pallet<T, I>,
+ Twox64Concat,
+ <T as frame_system::Config>::AccountId,
+ PreScoreNode<T, I>,
+ >;
+
+ #[frame_support::storage_alias]
+ pub type CounterForListNodes<T: crate::Config<I>, I: 'static> =
+ StorageValue<crate::Pallet<T, I>, u32, ValueQuery>;
+
+ #[frame_support::storage_alias]
+ pub type TempStorage<T: crate::Config<I>, I: 'static> =
+ StorageValue<crate::Pallet<T, I>, u32, ValueQuery>;
+}
+
+/// A struct that migrates all bags lists to contain a score value.
+pub struct AddScore<T: crate::Config<I>, I: 'static = ()>(sp_std::marker::PhantomData<(T, I)>);
+impl<T: crate::Config<I>, I: 'static> OnRuntimeUpgrade for AddScore<T, I> {
+ #[cfg(feature = "try-runtime")]
+ fn pre_upgrade() -> Result<(), &'static str> {
+ // The list node data should be corrupt at this point, so this is zero.
+ ensure!(crate::ListNodes::<T, I>::iter().count() == 0, "list node data is not corrupt");
+ // We can use the helper `old::ListNode` to get the existing data.
+ let iter_node_count: u32 = old::ListNodes::<T, I>::iter().count() as u32;
+ let tracked_node_count: u32 = old::CounterForListNodes::<T, I>::get();
+ crate::log!(info, "number of nodes before: {:?} {:?}", iter_node_count, tracked_node_count);
+ ensure!(iter_node_count == tracked_node_count, "Node count is wrong.");
+ old::TempStorage::<T, I>::put(iter_node_count);
+ Ok(())
+ }
+
+ fn on_runtime_upgrade() -> frame_support::weights::Weight {
+ for (_key, node) in old::ListNodes::<T, I>::iter() {
+ let score = T::ScoreProvider::score(&node.id);
+
+ let new_node = crate::Node {
+ id: node.id.clone(),
+ prev: node.prev,
+ next: node.next,
+ bag_upper: node.bag_upper,
+ score,
+ _phantom: node._phantom,
+ };
+
+ crate::ListNodes::<T, I>::insert(node.id, new_node);
+ }
+
+ return frame_support::weights::Weight::MAX
+ }
+
+ #[cfg(feature = "try-runtime")]
+ fn post_upgrade() -> Result<(), &'static str> {
+ let node_count_before = old::TempStorage::<T, I>::take();
+ // Now, the list node data is not corrupt anymore.
+ let iter_node_count_after: u32 = crate::ListNodes::<T, I>::iter().count() as u32;
+ let tracked_node_count_after: u32 = crate::ListNodes::<T, I>::count();
+ crate::log!(
+ info,
+ "number of nodes after: {:?} {:?}",
+ iter_node_count_after,
+ tracked_node_count_after,
+ );
+ ensure!(iter_node_count_after == node_count_before, "Not all nodes were migrated.");
+ ensure!(tracked_node_count_after == iter_node_count_after, "Node count is wrong.");
+ Ok(())
+ }
+}
diff --git a/substrate/frame/bags-list/src/tests.rs b/substrate/frame/bags-list/src/tests.rs
index c63ab26d59a676d51f7493668ba575402962f03e..01c1642f882c1bce4a8ef59567b36d3d61c6a113 100644
--- a/substrate/frame/bags-list/src/tests.rs
+++ b/substrate/frame/bags-list/src/tests.rs
@@ -35,8 +35,10 @@ mod pallet {
);
// when increasing score to the level of non-existent bag
+ assert_eq!(List::<Runtime>::get_score(&42).unwrap(), 20);
StakingMock::set_score_of(&42, 2_000);
assert_ok!(BagsList::rebag(Origin::signed(0), 42));
+ assert_eq!(List::<Runtime>::get_score(&42).unwrap(), 2_000);
// then a new bag is created and the id moves into it
assert_eq!(
@@ -53,6 +55,8 @@ mod pallet {
List::<Runtime>::get_bags(),
vec![(10, vec![1]), (1_000, vec![2, 3, 4]), (2_000, vec![42])]
);
+ // but the score is updated
+ assert_eq!(List::<Runtime>::get_score(&42).unwrap(), 1_001);
// when reducing score to the level of a non-existent bag
StakingMock::set_score_of(&42, 30);
@@ -63,6 +67,7 @@ mod pallet {
List::<Runtime>::get_bags(),
vec![(10, vec![1]), (30, vec![42]), (1_000, vec![2, 3, 4])]
);
+ assert_eq!(List::<Runtime>::get_score(&42).unwrap(), 30);
// when increasing score to the level of a pre-existing bag
StakingMock::set_score_of(&42, 500);
@@ -73,6 +78,7 @@ mod pallet {
List::<Runtime>::get_bags(),
vec![(10, vec![1]), (1_000, vec![2, 3, 4, 42])]
);
+ assert_eq!(List::<Runtime>::get_score(&42).unwrap(), 500);
});
}
@@ -145,21 +151,20 @@ mod pallet {
}
#[test]
- fn wrong_rebag_is_noop() {
+ fn wrong_rebag_errs() {
ExtBuilder::default().build_and_execute(|| {
let node_3 = list::Node::<Runtime>::get(&3).unwrap();
- // when account 3 is _not_ misplaced with weight 500
+ // when account 3 is _not_ misplaced with score 500
NextVoteWeight::set(500);
assert!(!node_3.is_misplaced(500));
- // then calling rebag on account 3 with weight 500 is a noop
+ // then calling rebag on account 3 with score 500 is a noop
assert_storage_noop!(assert_eq!(BagsList::rebag(Origin::signed(0), 3), Ok(())));
// when account 42 is not in the list
assert!(!BagsList::contains(&42));
-
- // then rebag-ing account 42 is a noop
- assert_storage_noop!(assert_eq!(BagsList::rebag(Origin::signed(0), 42), Ok(())));
+ // then rebag-ing account 42 is an error
+ assert_storage_noop!(assert!(matches!(BagsList::rebag(Origin::signed(0), 42), Err(_))));
});
}
@@ -182,7 +187,6 @@ mod pallet {
#[test]
fn empty_threshold_works() {
BagThresholds::set(Default::default()); // which is the same as passing `()` to `Get<_>`.
-
ExtBuilder::default().build_and_execute(|| {
// everyone in the same bag.
assert_eq!(List::<Runtime>::get_bags(), vec![(VoteWeight::MAX, vec![1, 2, 3, 4])]);
@@ -196,8 +200,7 @@ mod pallet {
);
// any rebag is noop.
- assert_storage_noop!(assert!(BagsList::rebag(Origin::signed(0), 1).is_ok()));
- assert_storage_noop!(assert!(BagsList::rebag(Origin::signed(0), 10).is_ok()));
+ assert_storage_noop!(assert_eq!(BagsList::rebag(Origin::signed(0), 1), Ok(())));
})
}
diff --git a/substrate/frame/election-provider-support/src/lib.rs b/substrate/frame/election-provider-support/src/lib.rs
index af366d5b8f9192e78a9e84aae1b05772d20fd483..27bf7a37f96220a1e79102b9507e5d4d28912b9e 100644
--- a/substrate/frame/election-provider-support/src/lib.rs
+++ b/substrate/frame/election-provider-support/src/lib.rs
@@ -168,15 +168,12 @@
pub mod onchain;
pub mod traits;
-#[cfg(feature = "std")]
-use codec::{Decode, Encode};
-use frame_support::{weights::Weight, BoundedVec, RuntimeDebug};
use sp_runtime::traits::{Bounded, Saturating, Zero};
use sp_std::{fmt::Debug, prelude::*};
/// Re-export the solution generation macro.
pub use frame_election_provider_solution_type::generate_solution_type;
-pub use frame_support::traits::Get;
+pub use frame_support::{traits::Get, weights::Weight, BoundedVec, RuntimeDebug};
/// Re-export some type as they are used in the interface.
pub use sp_arithmetic::PerThing;
pub use sp_npos_elections::{
@@ -223,7 +220,7 @@ impl<T> __OrInvalidIndex<T> for Option<T> {
/// making it fast to repeatedly encode into a `SolutionOf<T>`. This property turns out
/// to be important when trimming for solution length.
#[derive(RuntimeDebug, Clone, Default)]
-#[cfg_attr(feature = "std", derive(PartialEq, Eq, Encode, Decode))]
+#[cfg_attr(feature = "std", derive(PartialEq, Eq, codec::Encode, codec::Decode))]
pub struct IndexAssignment<VoterIndex, TargetIndex, P: PerThing> {
/// Index of the voter among the voters list.
pub who: VoterIndex,
@@ -467,6 +464,29 @@ pub trait SortedListProvider<AccountId> {
/// Returns `Ok(())` iff it successfully updates an item, an `Err(_)` otherwise.
fn on_update(id: &AccountId, score: Self::Score) -> Result<(), Self::Error>;
+ /// Get the score of `id`.
+ fn get_score(id: &AccountId) -> Result<Self::Score, Self::Error>;
+
+ /// Same as `on_update`, but incorporate some increased score.
+ fn on_increase(id: &AccountId, additional: Self::Score) -> Result<(), Self::Error> {
+ let old_score = Self::get_score(id)?;
+ let new_score = old_score.saturating_add(additional);
+ Self::on_update(id, new_score)
+ }
+
+ /// Same as `on_update`, but incorporate some decreased score.
+ ///
+ /// If the new score of the item is `Zero`, it is removed.
+ fn on_decrease(id: &AccountId, decreased: Self::Score) -> Result<(), Self::Error> {
+ let old_score = Self::get_score(id)?;
+ let new_score = old_score.saturating_sub(decreased);
+ if new_score.is_zero() {
+ Self::on_remove(id)
+ } else {
+ Self::on_update(id, new_score)
+ }
+ }
+
/// Hook for removing am id from the list.
///
/// Returns `Ok(())` iff it successfully removes an item, an `Err(_)` otherwise.
@@ -504,7 +524,7 @@ pub trait SortedListProvider<AccountId> {
}
}
-/// Something that can provide the `VoteWeight` of an account. Similar to [`ElectionProvider`] and
+/// Something that can provide the `Score` of an account. Similar to [`ElectionProvider`] and
/// [`ElectionDataProvider`], this should typically be implementing by whoever is supposed to *use*
/// `SortedListProvider`.
pub trait ScoreProvider<AccountId> {
@@ -513,7 +533,7 @@ pub trait ScoreProvider<AccountId> {
/// Get the current `Score` of `who`.
fn score(who: &AccountId) -> Self::Score;
- /// For tests and benchmarks, set the `VoteWeight`.
+ /// For tests and benchmarks, set the `score`.
#[cfg(any(feature = "runtime-benchmarks", test))]
fn set_score_of(_: &AccountId, _: Self::Score) {}
}
diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs
index 34d7a176670f2abd3b0df5029becf62a31ac589d..4de1ea36cb591d354ea0acbc869916e144c323e4 100644
--- a/substrate/frame/staking/src/pallet/impls.rs
+++ b/substrate/frame/staking/src/pallet/impls.rs
@@ -1343,7 +1343,10 @@ impl<T: Config> SortedListProvider<T::AccountId> for UseNominatorsAndValidatorsM
// nothing to do on insert.
Ok(())
}
- fn on_update(_: &T::AccountId, _weight: VoteWeight) -> Result<(), Self::Error> {
+ fn get_score(id: &T::AccountId) -> Result<Self::Score, Self::Error> {
+ Ok(Pallet::<T>::weight_of(id))
+ }
+ fn on_update(_: &T::AccountId, _weight: Self::Score) -> Result<(), Self::Error> {
// nothing to do on update.
Ok(())
}