diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 9e77adaeee677..778aac7c4ede0 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -28,7 +28,7 @@ //! //! ## Key terms //! -//! * pool id: A unique identifier of each pool. Set to u12 +//! * pool id: A unique identifier of each pool. Set to u32. //! * bonded pool: Tracks the distribution of actively staked funds. See [`BondedPool`] and //! [`BondedPoolInner`]. //! * reward pool: Tracks rewards earned by actively staked funds. See [`RewardPool`] and @@ -47,7 +47,7 @@ //! exactly the same rules and conditions as a normal staker. Its bond increases or decreases as //! members join, it can `nominate` or `chill`, and might not even earn staking rewards if it is //! not nominating proper validators. -//! * reward account: A similar key-less account, that is set as the `Payee` account fo the bonded +//! * reward account: A similar key-less account, that is set as the `Payee` account for the bonded //! account for all staking rewards. //! //! ## Usage @@ -1451,6 +1451,10 @@ pub mod pallet { Defensive(DefensiveError), /// Partial unbonding now allowed permissionlessly. PartialUnbondNotAllowedPermissionlessly, + /// Pool id currently in use. + PoolIdInUse, + /// Pool id provided is not correct/usable. + InvalidPoolId, } #[derive(Encode, Decode, PartialEq, TypeInfo, frame_support::PalletError, RuntimeDebug)] @@ -1874,73 +1878,38 @@ pub mod pallet { nominator: AccountIdLookupOf, state_toggler: AccountIdLookupOf, ) -> DispatchResult { - let who = ensure_signed(origin)?; - let root = T::Lookup::lookup(root)?; - let nominator = T::Lookup::lookup(nominator)?; - let state_toggler = T::Lookup::lookup(state_toggler)?; - - ensure!(amount >= Pallet::::depositor_min_bond(), Error::::MinimumBondNotMet); - ensure!( - MaxPools::::get() - .map_or(true, |max_pools| BondedPools::::count() < max_pools), - Error::::MaxPools - ); - ensure!(!PoolMembers::::contains_key(&who), Error::::AccountBelongsToOtherPool); + let depositor = ensure_signed(origin)?; let pool_id = LastPoolId::::try_mutate::<_, Error, _>(|id| { *id = id.checked_add(1).ok_or(Error::::OverflowRisk)?; Ok(*id) })?; - let mut bonded_pool = BondedPool::::new( - pool_id, - PoolRoles { - root: Some(root), - nominator: Some(nominator), - state_toggler: Some(state_toggler), - depositor: who.clone(), - }, - ); - - bonded_pool.try_inc_members()?; - let points = bonded_pool.try_bond_funds(&who, amount, BondType::Create)?; - - T::Currency::transfer( - &who, - &bonded_pool.reward_account(), - T::Currency::minimum_balance(), - ExistenceRequirement::AllowDeath, - )?; - PoolMembers::::insert( - who.clone(), - PoolMember:: { - pool_id, - points, - last_recorded_reward_counter: Zero::zero(), - unbonding_eras: Default::default(), - }, - ); - RewardPools::::insert( - pool_id, - RewardPool:: { - last_recorded_reward_counter: Zero::zero(), - last_recorded_total_payouts: Zero::zero(), - total_rewards_claimed: Zero::zero(), - }, - ); - ReversePoolIdLookup::::insert(bonded_pool.bonded_account(), pool_id); + Self::do_create(depositor, amount, root, nominator, state_toggler, pool_id) + } - Self::deposit_event(Event::::Created { depositor: who.clone(), pool_id }); + /// Create a new delegation pool with a previously used pool id + /// + /// # Arguments + /// + /// same as `create` with the inclusion of + /// * `pool_id` - `A valid PoolId. + #[pallet::weight(T::WeightInfo::create())] + #[transactional] + pub fn create_with_pool_id( + origin: OriginFor, + #[pallet::compact] amount: BalanceOf, + root: AccountIdLookupOf, + nominator: AccountIdLookupOf, + state_toggler: AccountIdLookupOf, + pool_id: PoolId, + ) -> DispatchResult { + let depositor = ensure_signed(origin)?; - Self::deposit_event(Event::::Bonded { - member: who, - pool_id, - bonded: amount, - joined: true, - }); - bonded_pool.put(); + ensure!(!BondedPools::::contains_key(pool_id), Error::::PoolIdInUse); + ensure!(pool_id < LastPoolId::::get(), Error::::InvalidPoolId); - Ok(()) + Self::do_create(depositor, amount, root, nominator, state_toggler, pool_id) } /// Nominate on behalf of the pool. @@ -2364,6 +2333,76 @@ impl Pallet { Ok(pending_rewards) } + fn do_create( + who: T::AccountId, + amount: BalanceOf, + root: AccountIdLookupOf, + nominator: AccountIdLookupOf, + state_toggler: AccountIdLookupOf, + pool_id: PoolId, + ) -> DispatchResult { + let root = T::Lookup::lookup(root)?; + let nominator = T::Lookup::lookup(nominator)?; + let state_toggler = T::Lookup::lookup(state_toggler)?; + + ensure!(amount >= Pallet::::depositor_min_bond(), Error::::MinimumBondNotMet); + ensure!( + MaxPools::::get().map_or(true, |max_pools| BondedPools::::count() < max_pools), + Error::::MaxPools + ); + ensure!(!PoolMembers::::contains_key(&who), Error::::AccountBelongsToOtherPool); + let mut bonded_pool = BondedPool::::new( + pool_id, + PoolRoles { + root: Some(root), + nominator: Some(nominator), + state_toggler: Some(state_toggler), + depositor: who.clone(), + }, + ); + + bonded_pool.try_inc_members()?; + let points = bonded_pool.try_bond_funds(&who, amount, BondType::Create)?; + + T::Currency::transfer( + &who, + &bonded_pool.reward_account(), + T::Currency::minimum_balance(), + ExistenceRequirement::AllowDeath, + )?; + + PoolMembers::::insert( + who.clone(), + PoolMember:: { + pool_id, + points, + last_recorded_reward_counter: Zero::zero(), + unbonding_eras: Default::default(), + }, + ); + RewardPools::::insert( + pool_id, + RewardPool:: { + last_recorded_reward_counter: Zero::zero(), + last_recorded_total_payouts: Zero::zero(), + total_rewards_claimed: Zero::zero(), + }, + ); + ReversePoolIdLookup::::insert(bonded_pool.bonded_account(), pool_id); + + Self::deposit_event(Event::::Created { depositor: who.clone(), pool_id }); + + Self::deposit_event(Event::::Bonded { + member: who, + pool_id, + bonded: amount, + joined: true, + }); + bonded_pool.put(); + + Ok(()) + } + /// Ensure the correctness of the state of this pallet. /// /// This should be valid before or after each state transition of this pallet. diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 5074a7ffa695a..562371c856b7d 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -4198,6 +4198,44 @@ mod create { ); }); } + + #[test] + fn create_with_pool_id_works() { + ExtBuilder::default().build_and_execute(|| { + let ed = Balances::minimum_balance(); + + Balances::make_free_balance_be(&11, StakingMock::minimum_bond() + ed); + assert_ok!(Pools::create( + RuntimeOrigin::signed(11), + StakingMock::minimum_bond(), + 123, + 456, + 789 + )); + + assert_eq!(Balances::free_balance(&11), 0); + // delete the initial pool created, then pool_Id `1` will be free + + assert_noop!( + Pools::create_with_pool_id(RuntimeOrigin::signed(12), 20, 234, 654, 783, 1), + Error::::PoolIdInUse + ); + + assert_noop!( + Pools::create_with_pool_id(RuntimeOrigin::signed(12), 20, 234, 654, 783, 3), + Error::::InvalidPoolId + ); + + // start dismantling the pool. + assert_ok!(Pools::set_state(RuntimeOrigin::signed(902), 1, PoolState::Destroying)); + assert_ok!(fully_unbond_permissioned(10)); + + CurrentEra::set(3); + assert_ok!(Pools::withdraw_unbonded(RuntimeOrigin::signed(10), 10, 10)); + + assert_ok!(Pools::create_with_pool_id(RuntimeOrigin::signed(10), 20, 234, 654, 783, 1)); + }); + } } mod nominate {