From 1dd30eba61eea5b06356f198a3f823d78603f380 Mon Sep 17 00:00:00 2001 From: dharjeezy Date: Mon, 26 Aug 2024 14:29:30 +0100 Subject: [PATCH 1/5] try state check for points and rewards --- substrate/frame/staking/src/pallet/impls.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs index 2df3bc084eb0..5fd4606201ac 100644 --- a/substrate/frame/staking/src/pallet/impls.rs +++ b/substrate/frame/staking/src/pallet/impls.rs @@ -1982,7 +1982,8 @@ impl Pallet { Self::check_exposures()?; Self::check_paged_exposures()?; Self::check_count()?; - Self::ensure_disabled_validators_sorted() + Self::ensure_disabled_validators_sorted()?; + Self::check_points_and_rewards() } /// Invariants: @@ -2304,4 +2305,15 @@ impl Pallet { ); Ok(()) } + + fn check_points_and_rewards() -> Result<(), TryRuntimeError> { + let total_points = ErasRewardPoints::::iter().count() as u32; + let total_rewards = ErasTotalStake::::iter().count() as u32; + + ensure!( + total_points <= total_rewards, + "Points can never be lower than the stake in the pool" + ); + Ok(()) + } } From c773af406cdf031107185bd39117beda7ebb791f Mon Sep 17 00:00:00 2001 From: dharjeezy Date: Mon, 26 Aug 2024 14:41:30 +0100 Subject: [PATCH 2/5] pr doc and nit --- prdoc/pr_5465.prdoc | 10 ++++++++++ substrate/frame/staking/src/pallet/impls.rs | 8 +++++--- 2 files changed, 15 insertions(+), 3 deletions(-) create mode 100644 prdoc/pr_5465.prdoc diff --git a/prdoc/pr_5465.prdoc b/prdoc/pr_5465.prdoc new file mode 100644 index 000000000000..c81b7c2692de --- /dev/null +++ b/prdoc/pr_5465.prdoc @@ -0,0 +1,10 @@ +title: try-state check invariant for nomination-pools (points <= stake) + +doc: + - audience: Runtime Dev + description: | + Invariants `ErasRewardPoints` should be less than or equals `ErasTotalStake` + +crates: + - name: pallet-staking + bump: minor diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs index 5fd4606201ac..785988a6f791 100644 --- a/substrate/frame/staking/src/pallet/impls.rs +++ b/substrate/frame/staking/src/pallet/impls.rs @@ -2306,13 +2306,15 @@ impl Pallet { Ok(()) } + /// Invariants: + /// * Checks that `ErasRewardPoints` should be less than or equals `ErasTotalStake`. fn check_points_and_rewards() -> Result<(), TryRuntimeError> { let total_points = ErasRewardPoints::::iter().count() as u32; - let total_rewards = ErasTotalStake::::iter().count() as u32; + let total_stake = ErasTotalStake::::iter().count() as u32; ensure!( - total_points <= total_rewards, - "Points can never be lower than the stake in the pool" + total_points <= total_stake, + "Points should be less than or equals the stake in the pool" ); Ok(()) } From 0a52bd2330a347f9309a6adccba587c11aea0848 Mon Sep 17 00:00:00 2001 From: dharjeezy Date: Tue, 27 Aug 2024 11:25:23 +0100 Subject: [PATCH 3/5] correct implementation on nomination pools --- prdoc/pr_5465.prdoc | 6 +++--- substrate/frame/nomination-pools/src/lib.rs | 7 +++++++ substrate/frame/staking/src/pallet/impls.rs | 16 +--------------- 3 files changed, 11 insertions(+), 18 deletions(-) diff --git a/prdoc/pr_5465.prdoc b/prdoc/pr_5465.prdoc index c81b7c2692de..1edeb7fb1b86 100644 --- a/prdoc/pr_5465.prdoc +++ b/prdoc/pr_5465.prdoc @@ -1,10 +1,10 @@ -title: try-state check invariant for nomination-pools (points <= stake) +title: try-state check invariant for nomination-pools (points >= stake) doc: - audience: Runtime Dev description: | - Invariants `ErasRewardPoints` should be less than or equals `ErasTotalStake` + The Invariant holds the Each Bonded pool's point must never be lower than the stake in the pool crates: - - name: pallet-staking + - name: pallet-nomination-pools bump: minor diff --git a/substrate/frame/nomination-pools/src/lib.rs b/substrate/frame/nomination-pools/src/lib.rs index 44e3463dc9f2..955232695122 100644 --- a/substrate/frame/nomination-pools/src/lib.rs +++ b/substrate/frame/nomination-pools/src/lib.rs @@ -3658,6 +3658,7 @@ impl Pallet { /// * each `member.pool_id` must correspond to an existing `BondedPool.id` (which implies the /// existence of the reward pool as well). /// * count of all members must be less than `MaxPoolMembers`. + /// * each `BondedPool.points` must never be lower than the pool's balance. /// /// Then, considering unbonding members: /// @@ -3786,6 +3787,12 @@ impl Pallet { pool is being destroyed and the depositor is the last member", ); + ensure!( + bonded_pool.points >= + bonded_pool.points_to_balance(bonded_pool.points), + "Each `BondedPool.points` must never be lower than the pool's balance" + ); + expected_tvl += T::StakeAdapter::total_stake(Pool::from(bonded_pool.bonded_account())); Ok(()) diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs index 785988a6f791..2df3bc084eb0 100644 --- a/substrate/frame/staking/src/pallet/impls.rs +++ b/substrate/frame/staking/src/pallet/impls.rs @@ -1982,8 +1982,7 @@ impl Pallet { Self::check_exposures()?; Self::check_paged_exposures()?; Self::check_count()?; - Self::ensure_disabled_validators_sorted()?; - Self::check_points_and_rewards() + Self::ensure_disabled_validators_sorted() } /// Invariants: @@ -2305,17 +2304,4 @@ impl Pallet { ); Ok(()) } - - /// Invariants: - /// * Checks that `ErasRewardPoints` should be less than or equals `ErasTotalStake`. - fn check_points_and_rewards() -> Result<(), TryRuntimeError> { - let total_points = ErasRewardPoints::::iter().count() as u32; - let total_stake = ErasTotalStake::::iter().count() as u32; - - ensure!( - total_points <= total_stake, - "Points should be less than or equals the stake in the pool" - ); - Ok(()) - } } From bb2560b3b707498e85b653682eb75cb1cd778350 Mon Sep 17 00:00:00 2001 From: dharjeezy Date: Tue, 27 Aug 2024 11:26:08 +0100 Subject: [PATCH 4/5] fmt --- substrate/frame/nomination-pools/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/substrate/frame/nomination-pools/src/lib.rs b/substrate/frame/nomination-pools/src/lib.rs index 955232695122..177c5da74d4f 100644 --- a/substrate/frame/nomination-pools/src/lib.rs +++ b/substrate/frame/nomination-pools/src/lib.rs @@ -3788,8 +3788,7 @@ impl Pallet { ); ensure!( - bonded_pool.points >= - bonded_pool.points_to_balance(bonded_pool.points), + bonded_pool.points >= bonded_pool.points_to_balance(bonded_pool.points), "Each `BondedPool.points` must never be lower than the pool's balance" ); From 2a21ce9abc6b210b26ee30a3db988f08ba63b23f Mon Sep 17 00:00:00 2001 From: dharjeezy Date: Tue, 27 Aug 2024 15:40:19 +0100 Subject: [PATCH 5/5] Update prdoc/pr_5465.prdoc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gonçalo Pestana --- prdoc/pr_5465.prdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prdoc/pr_5465.prdoc b/prdoc/pr_5465.prdoc index 1edeb7fb1b86..ae185dc250f6 100644 --- a/prdoc/pr_5465.prdoc +++ b/prdoc/pr_5465.prdoc @@ -3,7 +3,7 @@ title: try-state check invariant for nomination-pools (points >= stake) doc: - audience: Runtime Dev description: | - The Invariant holds the Each Bonded pool's point must never be lower than the stake in the pool + Adds a new try-state invariant to the nomination pools that checks that for each bonded pool, the pool's points can never be lower than its staked balance. crates: - name: pallet-nomination-pools