diff --git a/frame/bags-list/src/benchmarks.rs b/frame/bags-list/src/benchmarks.rs index 800e668324e0c..41c65978e0d4f 100644 --- a/frame/bags-list/src/benchmarks.rs +++ b/frame/bags-list/src/benchmarks.rs @@ -19,7 +19,7 @@ use super::*; use crate::list::List; -use frame_benchmarking::{account, whitelisted_caller}; +use frame_benchmarking::{account, whitelist_account, whitelisted_caller}; use frame_election_provider_support::VoteWeightProvider; use frame_support::{assert_ok, traits::Get}; use frame_system::RawOrigin as SystemOrigin; @@ -137,9 +137,41 @@ frame_benchmarking::benchmarks! { ); } - impl_benchmark_test_suite!( - Pallet, - crate::mock::ExtBuilder::default().build(), - crate::mock::Runtime, - ) + 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::::insert(lighter.clone(), bag_thresh)); + + let heavier_prev: T::AccountId = account("heavier_prev", 0, 0); + assert_ok!(List::::insert(heavier_prev.clone(), bag_thresh)); + + let heavier: T::AccountId = account("heavier", 0, 0); + assert_ok!(List::::insert(heavier.clone(), bag_thresh)); + + let heavier_next: T::AccountId = account("heavier_next", 0, 0); + assert_ok!(List::::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::::iter().map(|n| n.id().clone()).collect::>(), + vec![lighter.clone(), heavier_prev.clone(), heavier.clone(), heavier_next.clone()] + ); + + whitelist_account!(heavier); + }: _(SystemOrigin::Signed(heavier.clone()), lighter.clone()) + verify { + assert_eq!( + List::::iter().map(|n| n.id().clone()).collect::>(), + vec![heavier, lighter, heavier_prev, heavier_next] + ) + } } diff --git a/frame/bags-list/src/lib.rs b/frame/bags-list/src/lib.rs index d3be2c29533f9..193a334cf08f6 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -173,6 +173,17 @@ pub mod pallet { Rebagged { who: T::AccountId, from: VoteWeight, to: VoteWeight }, } + #[pallet::error] + #[cfg_attr(test, derive(PartialEq))] + pub enum Error { + /// 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, + } + #[pallet::call] impl Pallet { /// Declare that some `dislocated` account has, through rewards or penalties, sufficiently @@ -190,6 +201,20 @@ pub mod pallet { let _ = Pallet::::do_rebag(&dislocated, current_weight); Ok(()) } + + /// Move the caller's Id directly in front of `lighter`. + /// + /// The dispatch origin for this call must be _Signed_ and can only be called by the Id of + /// the account going in front of `lighter`. + /// + /// Only works if + /// - both nodes are within the same bag, + /// - and `origin` has a greater `VoteWeight` than `lighter`. + #[pallet::weight(T::WeightInfo::put_in_front_of())] + pub fn put_in_front_of(origin: OriginFor, lighter: T::AccountId) -> DispatchResult { + let heavier = ensure_signed(origin)?; + List::::put_in_front_of(&lighter, &heavier).map_err(Into::into) + } } #[pallet::hooks] diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index 1ec4996c26fd3..4524101d793cf 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -376,6 +376,84 @@ impl List { }) } + /// Put `heavier_id` to the position directly in front of `lighter_id`. Both ids must be in the + /// 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, + ) -> Result<(), crate::pallet::Error> { + use crate::pallet; + use frame_support::ensure; + + let lighter_node = Node::::get(&lighter_id).ok_or(pallet::Error::IdNotFound)?; + let heavier_node = Node::::get(&heavier_id).ok_or(pallet::Error::IdNotFound)?; + + ensure!(lighter_node.bag_upper == heavier_node.bag_upper, pallet::Error::NotInSameBag); + + // this is the most expensive check, so we do it last. + ensure!( + T::VoteWeightProvider::vote_weight(&heavier_id) > + T::VoteWeightProvider::vote_weight(&lighter_id), + pallet::Error::NotHeavier + ); + + // remove the heavier node from this list. Note that this removes the node from storage and + // decrements the node counter. + Self::remove(&heavier_id); + + // re-fetch `lighter_node` from storage since it may have been updated when `heavier_node` + // was removed. + let lighter_node = Node::::get(&lighter_id).ok_or_else(|| { + debug_assert!(false, "id that should exist cannot be found"); + crate::log!(warn, "id that should exist cannot be found"); + pallet::Error::IdNotFound + })?; + + // insert `heavier_node` directly in front of `lighter_node`. This will update both nodes + // in storage and update the node counter. + Self::insert_at_unchecked(lighter_node, heavier_node); + + Ok(()) + } + + /// Insert `node` directly in front of `at`. + /// + /// WARNINGS: + /// - this is a naive function in that it does not check if `node` belongs to the same bag as + /// `at`. It is expected that the call site will check preconditions. + /// - this will panic if `at.bag_upper` is not a bag that already exists in storage. + fn insert_at_unchecked(mut at: Node, mut node: Node) { + // connect `node` to its new `prev`. + node.prev = at.prev.clone(); + if let Some(mut prev) = at.prev() { + prev.next = Some(node.id().clone()); + prev.put() + } + + // connect `node` and `at`. + node.next = Some(at.id().clone()); + at.prev = Some(node.id().clone()); + + if node.is_terminal() { + // `node` is the new head, so we make sure the bag is updated. Note, + // 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::::get(at.bag_upper) + .expect("given nodes must always have a valid bag. qed."); + + if node.prev == None { + bag.head = Some(node.id().clone()) + } + + bag.put() + }; + + // write the updated nodes to storage. + at.put(); + node.put(); + } + /// Sanity check the list. /// /// This should be called from the call-site, whenever one of the mutating apis (e.g. `insert`) diff --git a/frame/bags-list/src/list/tests.rs b/frame/bags-list/src/list/tests.rs index 1c345df9a2fbd..f3043589681ec 100644 --- a/frame/bags-list/src/list/tests.rs +++ b/frame/bags-list/src/list/tests.rs @@ -383,6 +383,123 @@ mod list { assert!(non_existent_ids.iter().all(|id| !List::::contains(id))); }) } + + #[test] + #[should_panic = "given nodes must always have a valid bag. qed."] + fn put_in_front_of_panics_if_bag_not_found() { + ExtBuilder::default().skip_genesis_ids().build_and_execute_no_post_check(|| { + let node_10_no_bag = Node:: { id: 10, prev: None, next: None, bag_upper: 15 }; + let node_11_no_bag = Node:: { id: 11, prev: None, next: None, bag_upper: 15 }; + + // given + ListNodes::::insert(10, node_10_no_bag); + ListNodes::::insert(11, node_11_no_bag); + StakingMock::set_vote_weight_of(&10, 14); + StakingMock::set_vote_weight_of(&11, 15); + assert!(!ListBags::::contains_key(15)); + assert_eq!(List::::get_bags(), vec![]); + + // then .. this panics + let _ = List::::put_in_front_of(&10, &11); + }); + } + + #[test] + fn insert_at_unchecked_at_is_only_node() { + // Note that this `insert_at_unchecked` test should fail post checks because node 42 does + // not get re-assigned the correct bagu pper. This is because `insert_at_unchecked` assumes + // both nodes are already in the same bag with the correct bag upper. + ExtBuilder::default().build_and_execute_no_post_check(|| { + // given + assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])]); + + // implicitly also test that `node`'s `prev`/`next` are correctly re-assigned. + let node_42 = + Node:: { id: 42, prev: Some(1), next: Some(2), bag_upper: 1_000 }; + assert!(!crate::ListNodes::::contains_key(42)); + + let node_1 = crate::ListNodes::::get(&1).unwrap(); + + // when + List::::insert_at_unchecked(node_1, node_42); + + // then + assert_eq!( + List::::get_bags(), + vec![(10, vec![42, 1]), (1_000, vec![2, 3, 4])] + ); + }) + } + + #[test] + fn insert_at_unchecked_at_is_head() { + ExtBuilder::default().build_and_execute(|| { + // given + assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])]); + + // implicitly also test that `node`'s `prev`/`next` are correctly re-assigned. + let node_42 = Node:: { id: 42, prev: Some(4), next: None, bag_upper: 1_000 }; + assert!(!crate::ListNodes::::contains_key(42)); + + let node_2 = crate::ListNodes::::get(&2).unwrap(); + + // when + List::::insert_at_unchecked(node_2, node_42); + + // then + assert_eq!( + List::::get_bags(), + vec![(10, vec![1]), (1_000, vec![42, 2, 3, 4])] + ); + }) + } + + #[test] + fn insert_at_unchecked_at_is_non_terminal() { + ExtBuilder::default().build_and_execute(|| { + // given + assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])]); + + // implicitly also test that `node`'s `prev`/`next` are correctly re-assigned. + let node_42 = Node:: { id: 42, prev: None, next: Some(2), bag_upper: 1_000 }; + assert!(!crate::ListNodes::::contains_key(42)); + + let node_3 = crate::ListNodes::::get(&3).unwrap(); + + // when + List::::insert_at_unchecked(node_3, node_42); + + // then + assert_eq!( + List::::get_bags(), + vec![(10, vec![1]), (1_000, vec![2, 42, 3, 4])] + ); + }) + } + + #[test] + fn insert_at_unchecked_at_is_tail() { + ExtBuilder::default().build_and_execute(|| { + // given + assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])]); + + // implicitly also test that `node`'s `prev`/`next` are correctly re-assigned. + let node_42 = + Node:: { id: 42, prev: Some(42), next: Some(42), bag_upper: 1_000 }; + assert!(!crate::ListNodes::::contains_key(42)); + + let node_4 = crate::ListNodes::::get(&4).unwrap(); + + // when + List::::insert_at_unchecked(node_4, node_42); + + // then + assert_eq!( + List::::get_bags(), + vec![(10, vec![1]), (1_000, vec![2, 3, 42, 4])] + ); + }) + } } mod bags { diff --git a/frame/bags-list/src/mock.rs b/frame/bags-list/src/mock.rs index 45eb1d85abe3c..6545e563a3efe 100644 --- a/frame/bags-list/src/mock.rs +++ b/frame/bags-list/src/mock.rs @@ -21,28 +21,26 @@ use super::*; use crate::{self as bags_list}; use frame_election_provider_support::VoteWeight; use frame_support::parameter_types; +use std::collections::HashMap; pub type AccountId = u32; pub type Balance = u32; parameter_types! { + // Set the vote weight for any id who's weight has _not_ been set with `set_vote_weight_of`. pub static NextVoteWeight: VoteWeight = 0; + pub static NextVoteWeightMap: HashMap = Default::default(); } pub struct StakingMock; impl frame_election_provider_support::VoteWeightProvider for StakingMock { fn vote_weight(id: &AccountId) -> VoteWeight { - match id { - 710 => 15, - 711 => 16, - 712 => 2_000, // special cases used for migrate test - _ => NextVoteWeight::get(), - } + *NextVoteWeightMap::get().get(id).unwrap_or(&NextVoteWeight::get()) } + #[cfg(any(feature = "runtime-benchmarks", test))] - fn set_vote_weight_of(_: &AccountId, weight: VoteWeight) { - // we don't really keep a mapping, just set weight for everyone. - NextVoteWeight::set(weight) + fn set_vote_weight_of(id: &AccountId, weight: VoteWeight) { + NEXT_VOTE_WEIGHT_MAP.with(|m| m.borrow_mut().insert(id.clone(), weight)); } } @@ -103,9 +101,17 @@ pub(crate) const GENESIS_IDS: [(AccountId, VoteWeight); 4] = #[derive(Default)] pub struct ExtBuilder { ids: Vec<(AccountId, VoteWeight)>, + skip_genesis_ids: bool, } impl ExtBuilder { + /// Skip adding the default genesis ids to the list. + #[cfg(test)] + pub(crate) fn skip_genesis_ids(mut self) -> Self { + self.skip_genesis_ids = true; + self + } + /// Add some AccountIds to insert into `List`. #[cfg(test)] pub(crate) fn add_ids(mut self, ids: Vec<(AccountId, VoteWeight)>) -> Self { @@ -117,10 +123,17 @@ impl ExtBuilder { sp_tracing::try_init_simple(); let storage = frame_system::GenesisConfig::default().build_storage::().unwrap(); + let ids_with_weight: Vec<_> = if self.skip_genesis_ids { + self.ids.iter().collect() + } else { + GENESIS_IDS.iter().chain(self.ids.iter()).collect() + }; + let mut ext = sp_io::TestExternalities::from(storage); ext.execute_with(|| { - for (id, weight) in GENESIS_IDS.iter().chain(self.ids.iter()) { + for (id, weight) in ids_with_weight { frame_support::assert_ok!(List::::insert(*id, *weight)); + StakingMock::set_vote_weight_of(id, *weight); } }); diff --git a/frame/bags-list/src/tests.rs b/frame/bags-list/src/tests.rs index 270d25855ccd4..8f1ccacaf1171 100644 --- a/frame/bags-list/src/tests.rs +++ b/frame/bags-list/src/tests.rs @@ -15,7 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use frame_support::{assert_ok, assert_storage_noop, traits::IntegrityTest}; +use frame_support::{assert_noop, assert_ok, assert_storage_noop, traits::IntegrityTest}; use super::*; use frame_election_provider_support::SortedListProvider; @@ -35,7 +35,7 @@ mod pallet { ); // when increasing vote weight to the level of non-existent bag - NextVoteWeight::set(2_000); + StakingMock::set_vote_weight_of(&42, 2_000); assert_ok!(BagsList::rebag(Origin::signed(0), 42)); // then a new bag is created and the id moves into it @@ -45,7 +45,7 @@ mod pallet { ); // when decreasing weight within the range of the current bag - NextVoteWeight::set(1001); + StakingMock::set_vote_weight_of(&42, 1_001); assert_ok!(BagsList::rebag(Origin::signed(0), 42)); // then the id does not move @@ -55,7 +55,7 @@ mod pallet { ); // when reducing weight to the level of a non-existent bag - NextVoteWeight::set(30); + StakingMock::set_vote_weight_of(&42, 30); assert_ok!(BagsList::rebag(Origin::signed(0), 42)); // then a new bag is created and the id moves into it @@ -65,7 +65,7 @@ mod pallet { ); // when increasing weight to the level of a pre-existing bag - NextVoteWeight::set(500); + StakingMock::set_vote_weight_of(&42, 500); assert_ok!(BagsList::rebag(Origin::signed(0), 42)); // then the id moves into that bag @@ -85,7 +85,7 @@ mod pallet { assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])]); // when - NextVoteWeight::set(10); + StakingMock::set_vote_weight_of(&4, 10); assert_ok!(BagsList::rebag(Origin::signed(0), 4)); // then @@ -93,6 +93,7 @@ mod pallet { assert_eq!(Bag::::get(1_000).unwrap(), Bag::new(Some(2), Some(3), 1_000)); // when + StakingMock::set_vote_weight_of(&3, 10); assert_ok!(BagsList::rebag(Origin::signed(0), 3)); // then @@ -103,6 +104,7 @@ mod pallet { assert_eq!(get_list_as_ids(), vec![2u32, 1, 4, 3]); // when + StakingMock::set_vote_weight_of(&2, 10); assert_ok!(BagsList::rebag(Origin::signed(0), 2)); // then @@ -117,7 +119,7 @@ mod pallet { fn rebag_head_works() { ExtBuilder::default().build_and_execute(|| { // when - NextVoteWeight::set(10); + StakingMock::set_vote_weight_of(&2, 10); assert_ok!(BagsList::rebag(Origin::signed(0), 2)); // then @@ -125,6 +127,7 @@ mod pallet { assert_eq!(Bag::::get(1_000).unwrap(), Bag::new(Some(3), Some(4), 1_000)); // when + StakingMock::set_vote_weight_of(&3, 10); assert_ok!(BagsList::rebag(Origin::signed(0), 3)); // then @@ -132,6 +135,7 @@ mod pallet { assert_eq!(Bag::::get(1_000).unwrap(), Bag::new(Some(4), Some(4), 1_000)); // when + StakingMock::set_vote_weight_of(&4, 10); assert_ok!(BagsList::rebag(Origin::signed(0), 4)); // then @@ -196,6 +200,249 @@ mod pallet { assert_storage_noop!(assert!(BagsList::rebag(Origin::signed(0), 10).is_ok())); }) } + + #[test] + fn put_in_front_of_two_node_bag_heavier_is_tail() { + ExtBuilder::default() + .skip_genesis_ids() + .add_ids(vec![(10, 15), (11, 16)]) + .build_and_execute(|| { + // given + assert_eq!(List::::get_bags(), vec![(20, vec![10, 11])]); + + // when + assert_ok!(BagsList::put_in_front_of(Origin::signed(11), 10)); + + // then + assert_eq!(List::::get_bags(), vec![(20, vec![11, 10])]); + }); + } + + #[test] + fn put_in_front_of_two_node_bag_heavier_is_head() { + ExtBuilder::default() + .skip_genesis_ids() + .add_ids(vec![(11, 16), (10, 15)]) + .build_and_execute(|| { + // given + assert_eq!(List::::get_bags(), vec![(20, vec![11, 10])]); + + // when + assert_ok!(BagsList::put_in_front_of(Origin::signed(11), 10)); + + // then + assert_eq!(List::::get_bags(), vec![(20, vec![11, 10])]); + }); + } + + #[test] + fn put_in_front_of_non_terminal_nodes_heavier_behind() { + ExtBuilder::default().add_ids(vec![(5, 1_000)]).build_and_execute(|| { + // given + assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4, 5])]); + + StakingMock::set_vote_weight_of(&3, 999); + + // when + assert_ok!(BagsList::put_in_front_of(Origin::signed(4), 3)); + + // then + assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 4, 3, 5])]); + }); + } + + #[test] + fn put_in_front_of_non_terminal_nodes_heavier_in_front() { + ExtBuilder::default() + .add_ids(vec![(5, 1_000), (6, 1_000)]) + .build_and_execute(|| { + // given + assert_eq!( + List::::get_bags(), + vec![(10, vec![1]), (1_000, vec![2, 3, 4, 5, 6])] + ); + + StakingMock::set_vote_weight_of(&5, 999); + + // when + assert_ok!(BagsList::put_in_front_of(Origin::signed(3), 5)); + + // then + assert_eq!( + List::::get_bags(), + vec![(10, vec![1]), (1_000, vec![2, 4, 3, 5, 6])] + ); + }); + } + + #[test] + fn put_in_front_of_lighter_is_head_heavier_is_non_terminal() { + ExtBuilder::default().build_and_execute(|| { + // given + assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])]); + + StakingMock::set_vote_weight_of(&2, 999); + + // when + assert_ok!(BagsList::put_in_front_of(Origin::signed(3), 2)); + + // then + assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![3, 2, 4])]); + }); + } + + #[test] + fn put_in_front_of_heavier_is_tail_lighter_is_non_terminal() { + ExtBuilder::default().build_and_execute(|| { + // given + assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])]); + + StakingMock::set_vote_weight_of(&3, 999); + + // when + assert_ok!(BagsList::put_in_front_of(Origin::signed(4), 3)); + + // then + assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 4, 3])]); + }); + } + + #[test] + fn put_in_front_of_heavier_is_tail_lighter_is_head() { + ExtBuilder::default().add_ids(vec![(5, 1_000)]).build_and_execute(|| { + // given + assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4, 5])]); + + StakingMock::set_vote_weight_of(&2, 999); + + // when + assert_ok!(BagsList::put_in_front_of(Origin::signed(5), 2)); + + // then + assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![5, 2, 3, 4])]); + }); + } + + #[test] + fn put_in_front_of_heavier_is_head_lighter_is_not_terminal() { + ExtBuilder::default().add_ids(vec![(5, 1_000)]).build_and_execute(|| { + // given + assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4, 5])]); + + StakingMock::set_vote_weight_of(&4, 999); + + // when + BagsList::put_in_front_of(Origin::signed(2), 4).unwrap(); + + // then + assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![3, 2, 4, 5])]); + }); + } + + #[test] + fn put_in_front_of_lighter_is_tail_heavier_is_not_terminal() { + ExtBuilder::default().add_ids(vec![(5, 900)]).build_and_execute(|| { + // given + assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4, 5])]); + + // when + BagsList::put_in_front_of(Origin::signed(3), 5).unwrap(); + + // then + assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 4, 3, 5])]); + }); + } + + #[test] + fn put_in_front_of_lighter_is_tail_heavier_is_head() { + ExtBuilder::default().build_and_execute(|| { + // given + assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])]); + + StakingMock::set_vote_weight_of(&4, 999); + + // when + BagsList::put_in_front_of(Origin::signed(2), 4).unwrap(); + + // then + assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![3, 2, 4])]); + }); + } + + #[test] + fn put_in_front_of_errors_if_heavier_is_less_than_lighter() { + ExtBuilder::default().build_and_execute(|| { + // given + assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])]); + + StakingMock::set_vote_weight_of(&3, 999); + + // then + assert_noop!( + BagsList::put_in_front_of(Origin::signed(3), 2), + crate::pallet::Error::::NotHeavier + ); + }); + } + + #[test] + fn put_in_front_of_errors_if_heavier_is_equal_weight_to_lighter() { + ExtBuilder::default().build_and_execute(|| { + // given + assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])]); + + // then + assert_noop!( + BagsList::put_in_front_of(Origin::signed(3), 4), + crate::pallet::Error::::NotHeavier + ); + }); + } + + #[test] + fn put_in_front_of_errors_if_nodes_not_found() { + // `heavier` not found + ExtBuilder::default().build_and_execute(|| { + // given + assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])]); + + assert!(!ListNodes::::contains_key(5)); + + // then + assert_noop!( + BagsList::put_in_front_of(Origin::signed(5), 4), + crate::pallet::Error::::IdNotFound + ); + }); + + // `lighter` not found + ExtBuilder::default().build_and_execute(|| { + // given + assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])]); + + assert!(!ListNodes::::contains_key(5)); + + // then + assert_noop!( + BagsList::put_in_front_of(Origin::signed(4), 5), + crate::pallet::Error::::IdNotFound + ); + }); + } + + #[test] + fn put_in_front_of_errors_if_nodes_not_in_same_bag() { + ExtBuilder::default().build_and_execute(|| { + // given + assert_eq!(List::::get_bags(), vec![(10, vec![1]), (1_000, vec![2, 3, 4])]); + + // then + assert_noop!( + BagsList::put_in_front_of(Origin::signed(4), 1), + crate::pallet::Error::::NotInSameBag + ); + }); + } } mod sorted_list_provider { diff --git a/frame/bags-list/src/weights.rs b/frame/bags-list/src/weights.rs index 95d3dfa6eb989..7433c7ac564f7 100644 --- a/frame/bags-list/src/weights.rs +++ b/frame/bags-list/src/weights.rs @@ -18,7 +18,7 @@ //! Autogenerated weights for pallet_bags_list //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev -//! DATE: 2021-09-15, STEPS: `50`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2021-11-18, STEPS: `50`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]` //! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 128 // Executed Command: @@ -35,7 +35,6 @@ // --output=./frame/bags-list/src/weights.rs // --template=./.maintain/frame-weight-template.hbs - #![cfg_attr(rustfmt, rustfmt_skip)] #![allow(unused_parens)] #![allow(unused_imports)] @@ -47,6 +46,7 @@ use sp_std::marker::PhantomData; pub trait WeightInfo { fn rebag_non_terminal() -> Weight; fn rebag_terminal() -> Weight; + fn put_in_front_of() -> Weight; } /// Weights for pallet_bags_list using the Substrate node and recommended hardware. @@ -57,7 +57,7 @@ impl WeightInfo for SubstrateWeight { // Storage: BagsList ListNodes (r:4 w:4) // Storage: BagsList ListBags (r:1 w:1) fn rebag_non_terminal() -> Weight { - (74_175_000 as Weight) + (70_481_000 as Weight) .saturating_add(T::DbWeight::get().reads(7 as Weight)) .saturating_add(T::DbWeight::get().writes(5 as Weight)) } @@ -66,10 +66,20 @@ impl WeightInfo for SubstrateWeight { // Storage: BagsList ListNodes (r:3 w:3) // Storage: BagsList ListBags (r:2 w:2) fn rebag_terminal() -> Weight { - (73_305_000 as Weight) + (68_642_000 as Weight) .saturating_add(T::DbWeight::get().reads(7 as Weight)) .saturating_add(T::DbWeight::get().writes(5 as Weight)) } + // Storage: BagsList ListNodes (r:4 w:4) + // Storage: Staking Bonded (r:2 w:0) + // Storage: Staking Ledger (r:2 w:0) + // Storage: BagsList CounterForListNodes (r:1 w:1) + // Storage: BagsList ListBags (r:1 w:1) + fn put_in_front_of() -> Weight { + (82_341_000 as Weight) + .saturating_add(T::DbWeight::get().reads(10 as Weight)) + .saturating_add(T::DbWeight::get().writes(6 as Weight)) + } } // For backwards compatibility and tests @@ -79,7 +89,7 @@ impl WeightInfo for () { // Storage: BagsList ListNodes (r:4 w:4) // Storage: BagsList ListBags (r:1 w:1) fn rebag_non_terminal() -> Weight { - (74_175_000 as Weight) + (70_481_000 as Weight) .saturating_add(RocksDbWeight::get().reads(7 as Weight)) .saturating_add(RocksDbWeight::get().writes(5 as Weight)) } @@ -88,8 +98,18 @@ impl WeightInfo for () { // Storage: BagsList ListNodes (r:3 w:3) // Storage: BagsList ListBags (r:2 w:2) fn rebag_terminal() -> Weight { - (73_305_000 as Weight) + (68_642_000 as Weight) .saturating_add(RocksDbWeight::get().reads(7 as Weight)) .saturating_add(RocksDbWeight::get().writes(5 as Weight)) } + // Storage: BagsList ListNodes (r:4 w:4) + // Storage: Staking Bonded (r:2 w:0) + // Storage: Staking Ledger (r:2 w:0) + // Storage: BagsList CounterForListNodes (r:1 w:1) + // Storage: BagsList ListBags (r:1 w:1) + fn put_in_front_of() -> Weight { + (82_341_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(10 as Weight)) + .saturating_add(RocksDbWeight::get().writes(6 as Weight)) + } }