-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[Staking] Consolidate try state warnings #11649
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ea1df48
82af640
c75e62d
23bb026
33a1e37
cb6c8ee
8f058d5
14d51a7
20d085a
63e3b7f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| title: 'Consolidate try-state warnings into summary counts' | ||
| doc: | ||
| - audience: Runtime Dev | ||
| description: |- | ||
| Aggregates repetitive try-state warnings (min bond violations, pool ED imbalance, depositor | ||
| insufficient stake) into single summary lines with counts and up to 10 example accounts. | ||
| Reduces log noise from hundreds of expected per-item warnings on production runtimes. | ||
|
|
||
| Closes #11646. | ||
|
|
||
| crates: | ||
| - name: pallet-staking-async | ||
| bump: patch | ||
| - name: pallet-nomination-pools | ||
| bump: patch |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4008,7 +4008,13 @@ impl<T: Config> Pallet<T> { | |
| })?; | ||
|
|
||
| let mut expected_tvl: BalanceOf<T> = Default::default(); | ||
| let mut depositor_undermin: Vec<(PoolId, T::AccountId)> = Vec::new(); | ||
| let mut depositor_undermin_total: u32 = 0; | ||
| let mut total_pools: u32 = 0; | ||
| const MAX_EXAMPLES: usize = 10; | ||
|
|
||
| BondedPools::<T>::iter().try_for_each(|(id, inner)| -> Result<(), TryRuntimeError> { | ||
| total_pools += 1; | ||
| let bonded_pool = BondedPool { id, inner }; | ||
| ensure!( | ||
| pools_members.get(&id).copied().unwrap_or_default() == | ||
|
|
@@ -4026,8 +4032,12 @@ impl<T: Config> Pallet<T> { | |
| .is_destroying_and_only_depositor(depositor.active_points()) || | ||
| depositor.active_points() >= MinCreateBond::<T>::get(); | ||
| if !depositor_has_enough_stake { | ||
| depositor_undermin_total += 1; | ||
|
Ank4n marked this conversation as resolved.
|
||
| if depositor_undermin.len() < MAX_EXAMPLES { | ||
| depositor_undermin.push((id, bonded_pool.roles.depositor.clone())); | ||
| } | ||
| log!( | ||
| warn, | ||
| trace, | ||
| "pool {:?} has depositor {:?} with insufficient stake {:?}, minimum required is {:?}", | ||
| id, | ||
| bonded_pool.roles.depositor, | ||
|
|
@@ -4046,6 +4056,17 @@ impl<T: Config> Pallet<T> { | |
| Ok(()) | ||
| })?; | ||
|
|
||
| if depositor_undermin_total > 0 { | ||
| log!( | ||
| warn, | ||
| "{}/{} pools have depositor with insufficient stake, minimum required is {:?}. Examples: {:?}", | ||
| depositor_undermin_total, | ||
| total_pools, | ||
| MinCreateBond::<T>::get(), | ||
| depositor_undermin, | ||
| ); | ||
| } | ||
|
|
||
| ensure!( | ||
| MaxPoolMembers::<T>::get().map_or(true, |max| all_members <= max), | ||
| Error::<T>::MaxPoolMembers | ||
|
|
@@ -4108,17 +4129,25 @@ impl<T: Config> Pallet<T> { | |
| debug_assertions | ||
| ))] | ||
| pub fn check_ed_imbalance() -> Result<u32, DispatchError> { | ||
| let mut needs_adjust = 0; | ||
| let mut needs_adjust: u32 = 0; | ||
| let mut total_pools: u32 = 0; | ||
| let mut ed_examples: Vec<PoolId> = Vec::new(); | ||
| const MAX_EXAMPLES: usize = 10; | ||
|
|
||
| BondedPools::<T>::iter_keys().for_each(|id| { | ||
| total_pools += 1; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Same as above: I suggest setting
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No strong opinions but I don't think it matters much. We are looping anyways. |
||
| let reward_acc = Self::generate_reward_account(id); | ||
| let frozen_balance = | ||
| T::Currency::balance_frozen(&FreezeReason::PoolMinBalance.into(), &reward_acc); | ||
|
|
||
| let expected_frozen_balance = T::Currency::minimum_balance(); | ||
| if frozen_balance != expected_frozen_balance { | ||
| needs_adjust += 1; | ||
| if ed_examples.len() < MAX_EXAMPLES { | ||
| ed_examples.push(id); | ||
| } | ||
| log!( | ||
| warn, | ||
| trace, | ||
| "pool {:?} has incorrect ED frozen that can result from change in ED. Expected = {:?}, Actual = {:?}. Use `adjust_pool_deposit` to fix it", | ||
| id, | ||
| expected_frozen_balance, | ||
|
|
@@ -4127,6 +4156,17 @@ impl<T: Config> Pallet<T> { | |
| } | ||
| }); | ||
|
|
||
| if needs_adjust > 0 { | ||
| log!( | ||
| warn, | ||
| "{}/{} pools have incorrect ED frozen (expected {:?}). Use `adjust_pool_deposit` to fix. Examples: {:?}", | ||
| needs_adjust, | ||
| total_pools, | ||
| T::Currency::minimum_balance(), | ||
| ed_examples, | ||
| ); | ||
| } | ||
|
|
||
| Ok(needs_adjust) | ||
| } | ||
| /// Fully unbond the shares of `member`, when executed from `origin`. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit:
total_poolssimply equalsBondedPools::<T>::count(), so I suggest simply setting it to this value before the iteration rather than incrementing it by 1 (for extra clarity).