diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 1257aabb9d..40316cee9b 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -727,6 +727,7 @@ impl pallet_bags_list::Config for Runtime { parameter_types! { pub const PostUnbondPoolsWindow: u32 = 4; pub const NominationPoolsPalletId: PalletId = PalletId(*b"py/nopls"); + pub const MinPointsToBalance: u32 = 10; } use sp_runtime::traits::Convert; @@ -754,6 +755,7 @@ impl pallet_nomination_pools::Config for Runtime { type MaxMetadataLen = ConstU32<256>; type MaxUnbonding = ConstU32<8>; type PalletId = NominationPoolsPalletId; + type MinPointsToBalance = MinPointsToBalance; } parameter_types! { diff --git a/substrate/frame/nomination-pools/benchmarking/src/mock.rs b/substrate/frame/nomination-pools/benchmarking/src/mock.rs index d98bcb542f..eb884869f6 100644 --- a/substrate/frame/nomination-pools/benchmarking/src/mock.rs +++ b/substrate/frame/nomination-pools/benchmarking/src/mock.rs @@ -144,6 +144,7 @@ impl Convert for U256ToBalance { parameter_types! { pub static PostUnbondingPoolsWindow: u32 = 10; pub const PoolsPalletId: PalletId = PalletId(*b"py/nopls"); + pub const MinPointsToBalance: u32 = 10; } impl pallet_nomination_pools::Config for Runtime { @@ -157,6 +158,7 @@ impl pallet_nomination_pools::Config for Runtime { type MaxMetadataLen = ConstU32<256>; type MaxUnbonding = ConstU32<8>; type PalletId = PoolsPalletId; + type MinPointsToBalance = MinPointsToBalance; } impl crate::Config for Runtime {} diff --git a/substrate/frame/nomination-pools/src/lib.rs b/substrate/frame/nomination-pools/src/lib.rs index d68a6f09c3..cdc3bdaf34 100644 --- a/substrate/frame/nomination-pools/src/lib.rs +++ b/substrate/frame/nomination-pools/src/lib.rs @@ -740,16 +740,22 @@ impl BondedPool { // We checked for zero above .div(bonded_balance); + let min_points_to_balance = T::MinPointsToBalance::get(); + // Pool points can inflate relative to balance, but only if the pool is slashed. // If we cap the ratio of points:balance so one cannot join a pool that has been slashed - // 90%, - ensure!(points_to_balance_ratio_floor < 10u32.into(), Error::::OverflowRisk); - // while restricting the balance to 1/10th of max total issuance, - let next_bonded_balance = bonded_balance.saturating_add(new_funds); + // by `min_points_to_balance`%, if not zero. ensure!( - next_bonded_balance < BalanceOf::::max_value().div(10u32.into()), + points_to_balance_ratio_floor < min_points_to_balance.into(), Error::::OverflowRisk ); + // while restricting the balance to `min_points_to_balance` of max total issuance, + let next_bonded_balance = bonded_balance.saturating_add(new_funds); + ensure!( + next_bonded_balance < BalanceOf::::max_value().div(min_points_to_balance.into()), + Error::::OverflowRisk + ); + // then we can be decently confident the bonding pool points will not overflow // `BalanceOf`. Note that these are just heuristics. @@ -1102,6 +1108,14 @@ pub mod pallet { #[pallet::constant] type PalletId: Get; + /// The minimum pool points-to-balance ratio that must be maintained for it to be `open`. + /// This is important in the event slashing takes place and the pool's points-to-balance + /// ratio becomes disproportional. + /// For a value of 10, the threshold would be a pool points-to-balance ratio of 10:1. + /// Such a scenario would also be the equivalent of the pool being 90% slashed. + #[pallet::constant] + type MinPointsToBalance: Get; + /// Infallible method for converting `Currency::Balance` to `U256`. type BalanceToU256: Convert, U256>; @@ -1844,7 +1858,7 @@ pub mod pallet { Ok(()) } - /// Update configurations for the nomination pools. The origin must for this call must be + /// Update configurations for the nomination pools. The origin for this call must be /// Root. /// /// # Arguments @@ -1880,7 +1894,6 @@ pub mod pallet { config_op_exp!(MaxPools::, max_pools); config_op_exp!(MaxPoolMembers::, max_members); config_op_exp!(MaxPoolMembersPerPool::, max_members_per_pool); - Ok(()) } @@ -1940,12 +1953,15 @@ pub mod pallet { #[pallet::hooks] impl Hooks> for Pallet { fn integrity_test() { + assert!( + T::MinPointsToBalance::get() > 0, + "Minimum points to balance ratio must be greater than 0" + ); assert!( sp_std::mem::size_of::() >= 2 * sp_std::mem::size_of::>(), "bit-length of the reward points must be at least twice as much as balance" ); - assert!( T::StakingInterface::bonding_duration() < TotalUnbondingPools::::get(), "There must be more unbonding pools then the bonding duration / @@ -2304,7 +2320,6 @@ impl Pallet { sum_unbonding_balance ); } - Ok(()) } diff --git a/substrate/frame/nomination-pools/src/mock.rs b/substrate/frame/nomination-pools/src/mock.rs index 5498496965..1dadd7e00c 100644 --- a/substrate/frame/nomination-pools/src/mock.rs +++ b/substrate/frame/nomination-pools/src/mock.rs @@ -162,6 +162,7 @@ parameter_types! { pub static MaxMetadataLen: u32 = 2; pub static CheckLevel: u8 = 255; pub const PoolsPalletId: PalletId = PalletId(*b"py/nopls"); + pub const MinPointsToBalance: u32 = 10; } impl pools::Config for Runtime { type Event = Event; @@ -174,6 +175,7 @@ impl pools::Config for Runtime { type PalletId = PoolsPalletId; type MaxMetadataLen = MaxMetadataLen; type MaxUnbonding = MaxUnbonding; + type MinPointsToBalance = MinPointsToBalance; } type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; diff --git a/substrate/frame/nomination-pools/src/tests.rs b/substrate/frame/nomination-pools/src/tests.rs index fe78da3bb1..c16c1c9da9 100644 --- a/substrate/frame/nomination-pools/src/tests.rs +++ b/substrate/frame/nomination-pools/src/tests.rs @@ -199,23 +199,34 @@ mod bonded_pool { }, }; + let min_points_to_balance: u128 = MinPointsToBalance::get().into(); + // Simulate a 100% slashed pool StakingMock::set_bonded_balance(pool.bonded_account(), 0); assert_noop!(pool.ok_to_join(0), Error::::OverflowRisk); - // Simulate a 89% - StakingMock::set_bonded_balance(pool.bonded_account(), 11); + // Simulate a slashed pool at `MinPointsToBalance` + 1 slashed pool + StakingMock::set_bonded_balance( + pool.bonded_account(), + min_points_to_balance.saturating_add(1).into(), + ); assert_ok!(pool.ok_to_join(0)); - // Simulate a 90% slashed pool - StakingMock::set_bonded_balance(pool.bonded_account(), 10); + // Simulate a slashed pool at `MinPointsToBalance` + StakingMock::set_bonded_balance(pool.bonded_account(), min_points_to_balance); assert_noop!(pool.ok_to_join(0), Error::::OverflowRisk); - StakingMock::set_bonded_balance(pool.bonded_account(), Balance::MAX / 10); - // New bonded balance would be over 1/10th of Balance type + StakingMock::set_bonded_balance( + pool.bonded_account(), + Balance::MAX / min_points_to_balance, + ); + // New bonded balance would be over threshold of Balance type assert_noop!(pool.ok_to_join(0), Error::::OverflowRisk); // and a sanity check - StakingMock::set_bonded_balance(pool.bonded_account(), Balance::MAX / 10 - 1); + StakingMock::set_bonded_balance( + pool.bonded_account(), + Balance::MAX / min_points_to_balance - 1, + ); assert_ok!(pool.ok_to_join(0)); }); } @@ -503,22 +514,36 @@ mod join { }, ); - // Force the points:balance ratio to 100/10 - StakingMock::set_bonded_balance(Pools::create_bonded_account(123), 10); + // Force the points:balance ratio to `MinPointsToBalance` (100/10) + let min_points_to_balance: u128 = MinPointsToBalance::get().into(); + + StakingMock::set_bonded_balance( + Pools::create_bonded_account(123), + min_points_to_balance, + ); assert_noop!(Pools::join(Origin::signed(11), 420, 123), Error::::OverflowRisk); - StakingMock::set_bonded_balance(Pools::create_bonded_account(123), Balance::MAX / 10); - // Balance is gt 1/10 of Balance::MAX + StakingMock::set_bonded_balance( + Pools::create_bonded_account(123), + Balance::MAX / min_points_to_balance, + ); + // Balance needs to be gt Balance::MAX / `MinPointsToBalance` assert_noop!(Pools::join(Origin::signed(11), 5, 123), Error::::OverflowRisk); - StakingMock::set_bonded_balance(Pools::create_bonded_account(1), 10); + StakingMock::set_bonded_balance(Pools::create_bonded_account(1), min_points_to_balance); // Cannot join a pool that isn't open unsafe_set_state(123, PoolState::Blocked).unwrap(); - assert_noop!(Pools::join(Origin::signed(11), 10, 123), Error::::NotOpen); + assert_noop!( + Pools::join(Origin::signed(11), min_points_to_balance, 123), + Error::::NotOpen + ); unsafe_set_state(123, PoolState::Destroying).unwrap(); - assert_noop!(Pools::join(Origin::signed(11), 10, 123), Error::::NotOpen); + assert_noop!( + Pools::join(Origin::signed(11), min_points_to_balance, 123), + Error::::NotOpen + ); // Given MinJoinBond::::put(100); @@ -3434,7 +3459,7 @@ mod set_configs { ConfigOp::Remove, ConfigOp::Remove, ConfigOp::Remove, - ConfigOp::Remove + ConfigOp::Remove, )); assert_eq!(MinJoinBond::::get(), 0); assert_eq!(MinCreateBond::::get(), 0);