Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
//! Autogenerated weights for `pallet_bags_list`
//!
//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 32.0.0
//! DATE: 2025-07-01, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]`
//! DATE: 2025-09-12, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]`
//! WORST CASE MAP SIZE: `1000000`
//! HOSTNAME: `66f1737e2c94`, CPU: `Intel(R) Xeon(R) CPU @ 2.60GHz`
//! HOSTNAME: `157b7e44410a`, CPU: `Intel(R) Xeon(R) CPU @ 2.60GHz`
//! WASM-EXECUTION: `Compiled`, CHAIN: `None`, DB CACHE: 1024

// Executed Command:
Expand Down Expand Up @@ -52,6 +52,8 @@ pub struct WeightInfo<T>(PhantomData<T>);
impl<T: frame_system::Config> pallet_bags_list::WeightInfo for WeightInfo<T> {
/// Storage: `VoterList::Lock` (r:1 w:0)
/// Proof: `VoterList::Lock` (`max_values`: Some(1), `max_size`: Some(0), added: 495, mode: `MaxEncodedLen`)
/// Storage: `VoterList::PendingRebag` (r:1 w:0)
/// Proof: `VoterList::PendingRebag` (`max_values`: None, `max_size`: Some(48), added: 2523, mode: `MaxEncodedLen`)
/// Storage: `VoterList::ListNodes` (r:4 w:4)
/// Proof: `VoterList::ListNodes` (`max_values`: None, `max_size`: Some(154), added: 2629, mode: `MaxEncodedLen`)
/// Storage: `Staking::Bonded` (r:1 w:0)
Expand All @@ -64,14 +66,16 @@ impl<T: frame_system::Config> pallet_bags_list::WeightInfo for WeightInfo<T> {
// Proof Size summary in bytes:
// Measured: `5012`
// Estimated: `11506`
// Minimum execution time: 149_768_000 picoseconds.
Weight::from_parts(162_271_000, 0)
// Minimum execution time: 154_457_000 picoseconds.
Weight::from_parts(161_546_000, 0)
.saturating_add(Weight::from_parts(0, 11506))
.saturating_add(T::DbWeight::get().reads(8))
.saturating_add(T::DbWeight::get().reads(9))
.saturating_add(T::DbWeight::get().writes(5))
}
/// Storage: `VoterList::Lock` (r:1 w:0)
/// Proof: `VoterList::Lock` (`max_values`: Some(1), `max_size`: Some(0), added: 495, mode: `MaxEncodedLen`)
/// Storage: `VoterList::PendingRebag` (r:1 w:0)
/// Proof: `VoterList::PendingRebag` (`max_values`: None, `max_size`: Some(48), added: 2523, mode: `MaxEncodedLen`)
/// Storage: `VoterList::ListNodes` (r:3 w:3)
/// Proof: `VoterList::ListNodes` (`max_values`: None, `max_size`: Some(154), added: 2629, mode: `MaxEncodedLen`)
/// Storage: `Staking::Bonded` (r:1 w:0)
Expand All @@ -84,10 +88,10 @@ impl<T: frame_system::Config> pallet_bags_list::WeightInfo for WeightInfo<T> {
// Proof Size summary in bytes:
// Measured: `4915`
// Estimated: `8877`
// Minimum execution time: 145_767_000 picoseconds.
Weight::from_parts(157_284_000, 0)
// Minimum execution time: 155_177_000 picoseconds.
Weight::from_parts(159_976_000, 0)
.saturating_add(Weight::from_parts(0, 8877))
.saturating_add(T::DbWeight::get().reads(8))
.saturating_add(T::DbWeight::get().reads(9))
.saturating_add(T::DbWeight::get().writes(5))
}
/// Storage: `VoterList::Lock` (r:1 w:0)
Expand All @@ -106,34 +110,38 @@ impl<T: frame_system::Config> pallet_bags_list::WeightInfo for WeightInfo<T> {
// Proof Size summary in bytes:
// Measured: `7365`
// Estimated: `11506`
// Minimum execution time: 192_992_000 picoseconds.
Weight::from_parts(200_226_000, 0)
// Minimum execution time: 190_977_000 picoseconds.
Weight::from_parts(206_642_000, 0)
.saturating_add(Weight::from_parts(0, 11506))
.saturating_add(T::DbWeight::get().reads(11))
.saturating_add(T::DbWeight::get().writes(6))
}
/// Storage: `VoterList::CounterForListNodes` (r:1 w:0)
/// Storage: `VoterList::CounterForListNodes` (r:1 w:1)
/// Proof: `VoterList::CounterForListNodes` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`)
/// Storage: `VoterList::CounterForPendingRebag` (r:1 w:1)
/// Proof: `VoterList::CounterForPendingRebag` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`)
/// Storage: `VoterList::Lock` (r:1 w:0)
/// Proof: `VoterList::Lock` (`max_values`: Some(1), `max_size`: Some(0), added: 495, mode: `MaxEncodedLen`)
/// Storage: `VoterList::NextNodeAutoRebagged` (r:1 w:1)
/// Proof: `VoterList::NextNodeAutoRebagged` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`)
/// Storage: `VoterList::ListBags` (r:200 w:4)
/// Storage: `VoterList::PendingRebag` (r:12 w:3)
/// Proof: `VoterList::PendingRebag` (`max_values`: None, `max_size`: Some(48), added: 2523, mode: `MaxEncodedLen`)
/// Storage: `VoterList::ListBags` (r:199 w:3)
/// Proof: `VoterList::ListBags` (`max_values`: None, `max_size`: Some(82), added: 2557, mode: `MaxEncodedLen`)
/// Storage: `VoterList::ListNodes` (r:11 w:11)
/// Storage: `VoterList::ListNodes` (r:11 w:10)
/// Proof: `VoterList::ListNodes` (`max_values`: None, `max_size`: Some(154), added: 2629, mode: `MaxEncodedLen`)
/// Storage: `Staking::Bonded` (r:10 w:0)
/// Proof: `Staking::Bonded` (`max_values`: None, `max_size`: Some(72), added: 2547, mode: `MaxEncodedLen`)
/// Storage: `Staking::Ledger` (r:10 w:0)
/// Storage: `Staking::Ledger` (r:9 w:0)
/// Proof: `Staking::Ledger` (`max_values`: None, `max_size`: Some(753), added: 3228, mode: `MaxEncodedLen`)
fn on_idle() -> Weight {
// Proof Size summary in bytes:
// Measured: `24300`
// Estimated: `512390`
// Minimum execution time: 1_064_149_000 picoseconds.
Weight::from_parts(1_090_878_000, 0)
.saturating_add(Weight::from_parts(0, 512390))
.saturating_add(T::DbWeight::get().reads(234))
.saturating_add(T::DbWeight::get().writes(16))
// Measured: `23853`
// Estimated: `509833`
// Minimum execution time: 1_173_907_000 picoseconds.
Weight::from_parts(1_196_347_000, 0)
.saturating_add(Weight::from_parts(0, 509833))
.saturating_add(T::DbWeight::get().reads(245))
.saturating_add(T::DbWeight::get().writes(19))
}
}
15 changes: 15 additions & 0 deletions prdoc/pr_9725.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
title: 'bags-list: implement pending rebag'
doc:
- audience: Runtime Dev
description: |-
Fix the issue where registering a nominator or validator while the bags list is locked would completely ignore the account, making it unreachable for on_idle rebagging.

Changes:
- Add PendingRebag storage to queue accounts that fail insertion during lock.
- Modify on_idle to process pending accounts with priority before regular accounts.
- Add cleanup logic for non-staking pending accounts.
crates:
- name: pallet-bags-list
bump: minor
- name: asset-hub-westend-runtime
bump: patch
1 change: 1 addition & 0 deletions substrate/frame/bags-list/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ pallet-balances = { default-features = true, workspace = true }
sp-core = { default-features = true, workspace = true }
sp-io = { default-features = true, workspace = true }
sp-tracing = { default-features = true, workspace = true }
substrate-test-utils = { workspace = true }

[features]
default = ["std"]
Expand Down
59 changes: 49 additions & 10 deletions substrate/frame/bags-list/src/benchmarks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,8 @@ benchmarks_instance_pallet! {
// This benchmark generates weights for `on_idle` based on runtime configuration.
// The main input is the runtime's `MaxAutoRebagPerBlock` type, which defines how many
// nodes can be rebagged per block.
// This benchmark simulates a more realistic and fragmented rebag scenario.
// This benchmark simulates a scenario with both pending rebag processing
// and fragmented rebag scenario.

List::<T, _>::unsafe_clear();

Expand All @@ -362,11 +363,14 @@ benchmarks_instance_pallet! {
let high = bag_thresh[2];

let rebag_budget = <T as Config<I>>::MaxAutoRebagPerBlock::get();
let n = rebag_budget + 5;

// Insert nodes with varying scores
for i in 0..n {
let node: T::AccountId = account("node", i, 0);
// Adjust counts to ensure exact budget usage
let pending_count = rebag_budget / 3; // Smaller portion for pending
let regular_count = rebag_budget + 5;

// Insert regular nodes with varying scores
for i in 0..regular_count {
let node: T::AccountId = account("regular_node", i, 0);
let score = match i % 3 {
0 => low - One::one(),
1 => mid - One::one(),
Expand All @@ -376,14 +380,38 @@ benchmarks_instance_pallet! {
}

// Corrupt some nodes to simulate edge cases
for i in (0..n).step_by(4) {
let node: T::AccountId = account("node", i, 0);
for i in (0..regular_count).step_by(4) {
let node: T::AccountId = account("regular_node", i, 0);
let _ = List::<T, _>::remove(&node); // orphan nodes
}

// Lock the list and simulate pending rebag insertions
<Pallet<T, I>>::lock();

// Create pending rebag entries (mix of valid and corrupted)
for i in 0..pending_count {
let pending_node: T::AccountId = account("pending_node", i, 0);
let pending_score = match i % 3 {
0 => mid,
1 => high,
_ => high + high,
};

// Set score first for most nodes, but skip some to simulate cleanup scenarios
if i % 7 != 0 {
T::ScoreProvider::set_score_of(&pending_node, pending_score);
}

let _ = <Pallet<T, I> as SortedListProvider<T::AccountId>>::on_insert(
pending_node, pending_score
);
}

<Pallet<T, I>>::unlock();

// Now set new scores that will move nodes into higher bags
for i in 0..n {
let node: T::AccountId = account("node", i, 0);
for i in 0..regular_count {
let node: T::AccountId = account("regular_node", i, 0);
let new_score = match i % 3 {
0 => mid,
1 => high,
Expand All @@ -392,6 +420,12 @@ benchmarks_instance_pallet! {
T::ScoreProvider::set_score_of(&node, new_score);
}

assert_eq!(
PendingRebag::<T, I>::count(),
pending_count,
"Expected exactly {} pending rebag entries",
pending_count
);
// Ensure we have at least three bags populated before rebag
assert!(List::<T, _>::get_bags().len() >= 2);
}
Expand All @@ -400,6 +434,11 @@ benchmarks_instance_pallet! {
<Pallet<T, I> as Hooks<_>>::on_idle(Default::default(), Weight::MAX);
}
verify {
// Verify all pending rebag entries were processed.
// This should always be true since pending_count = rebag_budget / 3 < rebag_budget,
// and pending accounts are processed first so all pending entries fit within the budget.
assert_eq!(PendingRebag::<T, I>::count(), 0, "All pending rebag entries should be processed");

// Count how many nodes ended up in higher bags
let total_rebagged: usize = List::<T, _>::get_bags()
.iter()
Expand All @@ -408,7 +447,7 @@ benchmarks_instance_pallet! {
.sum();

let expected = <T as Config<I>>::MaxAutoRebagPerBlock::get() as usize;
assert_eq!(total_rebagged, expected,"Expected exactly{:?} rebagged nodes, found {:?}", expected, total_rebagged);
assert_eq!(total_rebagged, expected, "Expected exactly {:?} rebagged nodes, found {:?}", expected, total_rebagged);
}

impl_benchmark_test_suite!(
Expand Down
Loading
Loading