Add MinPointsToBalance to nomination pools (paritytech#11377)

* add MinPointsToBalance to pools

* add min_points_to_balance to benchmark mock

* fmt

* comments

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* check min_points_to_balance.is_zero

* comment

* comment

* storage to constant

* fix

* comment

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: parity-processbot <>

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! {

frame/nomination-pools/benchmarking/src/mock.rs

@@ -144,6 +144,7 @@ impl Convert<sp_core::U256, Balance> 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 {}

frame/nomination-pools/src/lib.rs

@@ -740,16 +740,22 @@ impl<T: Config> BondedPool<T> {
 			// 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::<T>::OverflowRisk);
-		// while restricting the balance to 1/10th of max total issuance,
+		// by `min_points_to_balance`%, if not zero.
+		ensure!(
+			points_to_balance_ratio_floor < min_points_to_balance.into(),
+			Error::<T>::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::<T>::max_value().div(10u32.into()),
+			next_bonded_balance < BalanceOf::<T>::max_value().div(min_points_to_balance.into()),
 			Error::<T>::OverflowRisk
 		);
+
 		// then we can be decently confident the bonding pool points will not overflow
 		// `BalanceOf<T>`. Note that these are just heuristics.
 
@@ -1102,6 +1108,14 @@ pub mod pallet {
 		#[pallet::constant]
 		type PalletId: Get<frame_support::PalletId>;
 
+		/// 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<u32>;
+
 		/// Infallible method for converting `Currency::Balance` to `U256`.
 		type BalanceToU256: Convert<BalanceOf<Self>, 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::<T>, max_pools);
 			config_op_exp!(MaxPoolMembers::<T>, max_members);
 			config_op_exp!(MaxPoolMembersPerPool::<T>, max_members_per_pool);
-
 			Ok(())
 		}
 
@@ -1940,12 +1953,15 @@ pub mod pallet {
 	#[pallet::hooks]
 	impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
 		fn integrity_test() {
+			assert!(
+				T::MinPointsToBalance::get() > 0,
+				"Minimum points to balance ratio must be greater than 0"
+			);
 			assert!(
 				sp_std::mem::size_of::<RewardPoints>() >=
 					2 * sp_std::mem::size_of::<BalanceOf<T>>(),
 				"bit-length of the reward points must be at least twice as much as balance"
 			);
-
 			assert!(
 				T::StakingInterface::bonding_duration() < TotalUnbondingPools::<T>::get(),
 				"There must be more unbonding pools then the bonding duration /
@@ -2304,7 +2320,6 @@ impl<T: Config> Pallet<T> {
 				sum_unbonding_balance
 			);
 		}
-
 		Ok(())
 	}
 

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<Runtime>;

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::<Runtime>::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::<Runtime>::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::<Runtime>::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::<Runtime>::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::<Runtime>::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::<Runtime>::NotOpen);
+			assert_noop!(
+				Pools::join(Origin::signed(11), min_points_to_balance, 123),
+				Error::<Runtime>::NotOpen
+			);
 
 			unsafe_set_state(123, PoolState::Destroying).unwrap();
-			assert_noop!(Pools::join(Origin::signed(11), 10, 123), Error::<Runtime>::NotOpen);
+			assert_noop!(
+				Pools::join(Origin::signed(11), min_points_to_balance, 123),
+				Error::<Runtime>::NotOpen
+			);
 
 			// Given
 			MinJoinBond::<Runtime>::put(100);
@@ -3434,7 +3459,7 @@ mod set_configs {
 				ConfigOp::Remove,
 				ConfigOp::Remove,
 				ConfigOp::Remove,
-				ConfigOp::Remove
+				ConfigOp::Remove,
 			));
 			assert_eq!(MinJoinBond::<Runtime>::get(), 0);
 			assert_eq!(MinCreateBond::<Runtime>::get(), 0);