diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index aa446b03368d..a9fbbb4f33da 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -99,7 +99,7 @@ use sp_runtime::{ IdentityLookup, Keccak256, OpaqueKeys, SaturatedConversion, Verify, }, transaction_validity::{TransactionPriority, TransactionSource, TransactionValidity}, - ApplyExtrinsicResult, FixedU128, KeyTypeId, Perbill, Percent, Permill, + ApplyExtrinsicResult, FixedU128, KeyTypeId, Percent, Permill, }; use sp_staking::SessionIndex; #[cfg(any(feature = "std", test))] @@ -2476,6 +2476,14 @@ sp_api::impl_runtime_apis! { fn member_needs_delegate_migration(member: AccountId) -> bool { NominationPools::api_member_needs_delegate_migration(member) } + + fn member_total_balance(member: AccountId) -> Balance { + NominationPools::api_member_total_balance(member) + } + + fn pool_balance(pool_id: pallet_nomination_pools::PoolId) -> Balance { + NominationPools::api_pool_balance(pool_id) + } } impl pallet_staking_runtime_api::StakingApi for Runtime { @@ -2776,45 +2784,6 @@ sp_api::impl_runtime_apis! { } } -#[cfg(all(test, feature = "try-runtime"))] -mod remote_tests { - use super::*; - use frame_try_runtime::{runtime_decl_for_try_runtime::TryRuntime, UpgradeCheckSelect}; - use remote_externalities::{ - Builder, Mode, OfflineConfig, OnlineConfig, SnapshotConfig, Transport, - }; - use std::env::var; - - #[tokio::test] - async fn run_migrations() { - if var("RUN_MIGRATION_TESTS").is_err() { - return; - } - - sp_tracing::try_init_simple(); - let transport: Transport = - var("WS").unwrap_or("wss://westend-rpc.polkadot.io:443".to_string()).into(); - let maybe_state_snapshot: Option = var("SNAP").map(|s| s.into()).ok(); - let mut ext = Builder::::default() - .mode(if let Some(state_snapshot) = maybe_state_snapshot { - Mode::OfflineOrElseOnline( - OfflineConfig { state_snapshot: state_snapshot.clone() }, - OnlineConfig { - transport, - state_snapshot: Some(state_snapshot), - ..Default::default() - }, - ) - } else { - Mode::Online(OnlineConfig { transport, ..Default::default() }) - }) - .build() - .await - .unwrap(); - ext.execute_with(|| Runtime::on_runtime_upgrade(UpgradeCheckSelect::PreAndPost)); - } -} - mod clean_state_migration { use super::Runtime; #[cfg(feature = "try-runtime")] diff --git a/polkadot/runtime/westend/src/tests.rs b/polkadot/runtime/westend/src/tests.rs index 4d5e2e946bce..dc8103ab52c4 100644 --- a/polkadot/runtime/westend/src/tests.rs +++ b/polkadot/runtime/westend/src/tests.rs @@ -99,3 +99,140 @@ fn check_treasury_pallet_id() { westend_runtime_constants::TREASURY_PALLET_ID ); } + +#[cfg(all(test, feature = "try-runtime"))] +mod remote_tests { + use super::*; + use frame_try_runtime::{runtime_decl_for_try_runtime::TryRuntime, UpgradeCheckSelect}; + use remote_externalities::{ + Builder, Mode, OfflineConfig, OnlineConfig, SnapshotConfig, Transport, + }; + use std::env::var; + + #[tokio::test] + async fn run_migrations() { + if var("RUN_MIGRATION_TESTS").is_err() { + return; + } + + sp_tracing::try_init_simple(); + let transport: Transport = + var("WS").unwrap_or("wss://westend-rpc.polkadot.io:443".to_string()).into(); + let maybe_state_snapshot: Option = var("SNAP").map(|s| s.into()).ok(); + let mut ext = Builder::::default() + .mode(if let Some(state_snapshot) = maybe_state_snapshot { + Mode::OfflineOrElseOnline( + OfflineConfig { state_snapshot: state_snapshot.clone() }, + OnlineConfig { + transport, + state_snapshot: Some(state_snapshot), + ..Default::default() + }, + ) + } else { + Mode::Online(OnlineConfig { transport, ..Default::default() }) + }) + .build() + .await + .unwrap(); + ext.execute_with(|| Runtime::on_runtime_upgrade(UpgradeCheckSelect::PreAndPost)); + } + + #[tokio::test] + async fn delegate_stake_migration() { + // Intended to be run only manually. + if var("RUN_MIGRATION_TESTS").is_err() { + return; + } + use frame_support::assert_ok; + sp_tracing::try_init_simple(); + + let transport: Transport = var("WS").unwrap_or("ws://127.0.0.1:9900".to_string()).into(); + let maybe_state_snapshot: Option = var("SNAP").map(|s| s.into()).ok(); + let mut ext = Builder::::default() + .mode(if let Some(state_snapshot) = maybe_state_snapshot { + Mode::OfflineOrElseOnline( + OfflineConfig { state_snapshot: state_snapshot.clone() }, + OnlineConfig { + transport, + state_snapshot: Some(state_snapshot), + pallets: vec![ + "staking".into(), + "system".into(), + "balances".into(), + "nomination-pools".into(), + "delegated-staking".into(), + ], + ..Default::default() + }, + ) + } else { + Mode::Online(OnlineConfig { transport, ..Default::default() }) + }) + .build() + .await + .unwrap(); + ext.execute_with(|| { + // create an account with some balance + let alice = AccountId::from([1u8; 32]); + use frame_support::traits::Currency; + let _ = Balances::deposit_creating(&alice, 100_000 * UNITS); + + // iterate over all pools + pallet_nomination_pools::BondedPools::::iter_keys().for_each(|k| { + if pallet_nomination_pools::Pallet::::api_pool_needs_delegate_migration(k) + { + assert_ok!( + pallet_nomination_pools::Pallet::::migrate_pool_to_delegate_stake( + RuntimeOrigin::signed(alice.clone()).into(), + k, + ) + ); + } + }); + + // member migration stats + let mut success = 0; + let mut direct_stakers = 0; + let mut unexpected_errors = 0; + + // iterate over all pool members + pallet_nomination_pools::PoolMembers::::iter_keys().for_each(|k| { + if pallet_nomination_pools::Pallet::::api_member_needs_delegate_migration( + k.clone(), + ) { + // reasons migrations can fail: + let is_direct_staker = pallet_staking::Bonded::::contains_key(&k); + + let migration = pallet_nomination_pools::Pallet::::migrate_delegation( + RuntimeOrigin::signed(alice.clone()).into(), + sp_runtime::MultiAddress::Id(k.clone()), + ); + + if is_direct_staker { + // if the member is a direct staker, the migration should fail until pool + // member unstakes all funds from pallet-staking. + direct_stakers += 1; + assert_eq!( + migration.unwrap_err(), + pallet_delegated_staking::Error::::AlreadyStaking.into() + ); + } else if migration.is_err() { + unexpected_errors += 1; + log::error!(target: "remote_test", "Unexpected error {:?} while migrating {:?}", migration.unwrap_err(), k); + } else { + success += 1; + } + } + }); + + log::info!( + target: "remote_test", + "Migration stats: success: {}, direct_stakers: {}, unexpected_errors: {}", + success, + direct_stakers, + unexpected_errors + ); + }); + } +} diff --git a/prdoc/pr_4822.prdoc b/prdoc/pr_4822.prdoc new file mode 100644 index 000000000000..44f3e41d8d5a --- /dev/null +++ b/prdoc/pr_4822.prdoc @@ -0,0 +1,25 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: Ensure as many as possible pool members can migrate to `DelegateStake` + +doc: + - audience: Runtime Dev + description: | + 1. Allows pool members to use their total balance while joining pool with `DelegateStake`. + 2. Gates call mutating pool or member in unmigrated state. + 3. Runtime apis for reading pool and member balance. + +crates: + - name: westend-runtime + bump: minor + - name: kitchensink-runtime + bump: patch + - name: pallet-delegated-staking + bump: patch + - name: pallet-nomination-pools + bump: minor + - name: sp-staking + bump: patch + - name: pallet-nomination-pools-runtime-api + bump: minor diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index ff3f7e26baf2..ea9a66862d64 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -2784,6 +2784,14 @@ impl_runtime_apis! { fn member_needs_delegate_migration(member: AccountId) -> bool { NominationPools::api_member_needs_delegate_migration(member) } + + fn member_total_balance(member: AccountId) -> Balance { + NominationPools::api_member_total_balance(member) + } + + fn pool_balance(pool_id: pallet_nomination_pools::PoolId) -> Balance { + NominationPools::api_pool_balance(pool_id) + } } impl pallet_staking_runtime_api::StakingApi for Runtime { diff --git a/substrate/frame/delegated-staking/src/impls.rs b/substrate/frame/delegated-staking/src/impls.rs index 8c05b0bcfc22..4e6812dee249 100644 --- a/substrate/frame/delegated-staking/src/impls.rs +++ b/substrate/frame/delegated-staking/src/impls.rs @@ -19,7 +19,7 @@ //! Implementations of public traits, namely [`DelegationInterface`] and [`OnStakingUpdate`]. use super::*; -use sp_staking::{Agent, DelegationInterface, DelegationMigrator, Delegator, OnStakingUpdate}; +use sp_staking::{DelegationInterface, DelegationMigrator, OnStakingUpdate}; impl DelegationInterface for Pallet { type Balance = BalanceOf; diff --git a/substrate/frame/delegated-staking/src/lib.rs b/substrate/frame/delegated-staking/src/lib.rs index 1882989cfa7d..08ce747ec0c0 100644 --- a/substrate/frame/delegated-staking/src/lib.rs +++ b/substrate/frame/delegated-staking/src/lib.rs @@ -148,7 +148,7 @@ use frame_support::{ }, Balanced, Inspect as FunInspect, Mutate as FunMutate, }, - tokens::{fungible::Credit, Fortitude, Precision, Preservation}, + tokens::{fungible::Credit, Fortitude, Precision, Preservation, Restriction}, Defensive, DefensiveOption, Imbalance, OnUnbalanced, }, }; @@ -319,9 +319,7 @@ pub mod pallet { Error::::NotAllowed ); - // remove provider reference - let _ = frame_system::Pallet::::dec_providers(&who)?; - >::remove(who); + AgentLedger::::remove(&who); Ok(()) } @@ -392,9 +390,6 @@ pub mod pallet { ) -> DispatchResult { let agent = ensure_signed(origin)?; - // Ensure they have minimum delegation. - ensure!(amount >= T::Currency::minimum_balance(), Error::::NotEnoughFunds); - // Ensure delegator is sane. ensure!(!Self::is_agent(&delegator), Error::::NotAllowed); ensure!(!Self::is_delegator(&delegator), Error::::NotAllowed); @@ -493,13 +488,8 @@ impl Pallet { /// Registers a new agent in the system. fn do_register_agent(who: &T::AccountId, reward_account: &T::AccountId) { + // TODO: Consider taking a deposit for being an agent. AgentLedger::::new(reward_account).update(who); - - // Agent does not hold balance of its own but this pallet will provide for this to exist. - // This is expected to be a keyless account and not created by any user directly so safe. - // TODO: Someday if we allow anyone to be an agent, we should take a deposit for - // being a delegator. - frame_system::Pallet::::inc_providers(who); } /// Migrate existing staker account `who` to an `Agent` account. @@ -510,9 +500,6 @@ impl Pallet { // transferred to actual delegator. let proxy_delegator = Self::generate_proxy_delegator(Agent::from(who.clone())); - // Keep proxy delegator alive until all funds are migrated. - frame_system::Pallet::::inc_providers(&proxy_delegator.clone().get()); - // Get current stake let stake = T::CoreStaking::stake(who)?; @@ -534,7 +521,6 @@ impl Pallet { T::CoreStaking::set_payee(who, reward_account)?; // delegate all transferred funds back to agent. Self::do_delegate(proxy_delegator, Agent::from(who.clone()), amount_to_transfer)?; - // if the transferred/delegated amount was greater than the stake, mark the extra as // unclaimed withdrawal. let unclaimed_withdraws = amount_to_transfer @@ -578,21 +564,23 @@ impl Pallet { let delegator = delegator.get(); let mut ledger = AgentLedger::::get(&agent).ok_or(Error::::NotAgent)?; + + if let Some(mut existing_delegation) = Delegation::::get(&delegator) { + ensure!(existing_delegation.agent == agent, Error::::InvalidDelegation); + // update amount and return the updated delegation. + existing_delegation.amount = existing_delegation + .amount + .checked_add(&amount) + .ok_or(ArithmeticError::Overflow)?; + existing_delegation + } else { + Delegation::::new(&agent, amount) + } + .update(&delegator); + // try to hold the funds. T::Currency::hold(&HoldReason::StakingDelegation.into(), &delegator, amount)?; - let new_delegation_amount = - if let Some(existing_delegation) = Delegation::::get(&delegator) { - ensure!(existing_delegation.agent == agent, Error::::InvalidDelegation); - existing_delegation - .amount - .checked_add(&amount) - .ok_or(ArithmeticError::Overflow)? - } else { - amount - }; - - Delegation::::new(&agent, new_delegation_amount).update_or_kill(&delegator); ledger.total_delegated = ledger.total_delegated.checked_add(&amount).ok_or(ArithmeticError::Overflow)?; ledger.update(&agent); @@ -638,9 +626,6 @@ impl Pallet { .checked_sub(&amount) .defensive_ok_or(ArithmeticError::Overflow)?; - // remove delegator if nothing delegated anymore - delegation.update_or_kill(&delegator); - let released = T::Currency::release( &HoldReason::StakingDelegation.into(), &delegator, @@ -650,6 +635,9 @@ impl Pallet { defensive_assert!(released == amount, "hold should have been released fully"); + // update delegation. + delegation.update(&delegator); + Self::deposit_event(Event::::Released { agent, delegator, amount }); Ok(()) @@ -668,49 +656,34 @@ impl Pallet { let mut source_delegation = Delegators::::get(&source_delegator).defensive_ok_or(Error::::BadState)?; - // some checks that must have already been checked before. + // ensure source has enough funds to migrate. ensure!(source_delegation.amount >= amount, Error::::NotEnoughFunds); debug_assert!( !Self::is_delegator(&destination_delegator) && !Self::is_agent(&destination_delegator) ); let agent = source_delegation.agent.clone(); - // update delegations - Delegation::::new(&agent, amount).update_or_kill(&destination_delegator); + // create a new delegation for destination delegator. + Delegation::::new(&agent, amount).update(&destination_delegator); source_delegation.amount = source_delegation .amount .checked_sub(&amount) .defensive_ok_or(Error::::BadState)?; - source_delegation.update_or_kill(&source_delegator); - - // release funds from source - let released = T::Currency::release( + // transfer the held amount in `source_delegator` to `destination_delegator`. + let _ = T::Currency::transfer_on_hold( &HoldReason::StakingDelegation.into(), - &source_delegator, - amount, - Precision::BestEffort, - )?; - - defensive_assert!(released == amount, "hold should have been released fully"); - - // transfer the released amount to `destination_delegator`. - let post_balance = T::Currency::transfer( &source_delegator, &destination_delegator, amount, - Preservation::Expendable, - ) - .map_err(|_| Error::::BadState)?; - - // if balance is zero, clear provider for source (proxy) delegator. - if post_balance == Zero::zero() { - let _ = frame_system::Pallet::::dec_providers(&source_delegator).defensive(); - } + Precision::Exact, + Restriction::OnHold, + Fortitude::Polite, + )?; - // hold the funds again in the new delegator account. - T::Currency::hold(&HoldReason::StakingDelegation.into(), &destination_delegator, amount)?; + // update source delegation. + source_delegation.update(&source_delegator); Self::deposit_event(Event::::MigratedDelegation { agent, @@ -752,7 +725,7 @@ impl Pallet { agent_ledger.remove_slash(actual_slash).save(); delegation.amount = delegation.amount.checked_sub(&actual_slash).ok_or(ArithmeticError::Overflow)?; - delegation.update_or_kill(&delegator); + delegation.update(&delegator); if let Some(reporter) = maybe_reporter { let reward_payout: BalanceOf = T::SlashRewardFraction::get() * actual_slash; @@ -801,18 +774,21 @@ impl Pallet { ledgers: BTreeMap>, ) -> Result<(), sp_runtime::TryRuntimeError> { for (agent, ledger) in ledgers { - ensure!( - matches!( - T::CoreStaking::status(&agent).expect("agent should be bonded"), - sp_staking::StakerStatus::Nominator(_) | sp_staking::StakerStatus::Idle - ), - "agent should be bonded and not validator" - ); + let staked_value = ledger.stakeable_balance(); + + if !staked_value.is_zero() { + ensure!( + matches!( + T::CoreStaking::status(&agent).expect("agent should be bonded"), + sp_staking::StakerStatus::Nominator(_) | sp_staking::StakerStatus::Idle + ), + "agent should be bonded and not validator" + ); + } ensure!( ledger.stakeable_balance() >= - T::CoreStaking::total_stake(&agent) - .expect("agent should exist as a nominator"), + T::CoreStaking::total_stake(&agent).unwrap_or_default(), "Cannot stake more than balance" ); } diff --git a/substrate/frame/delegated-staking/src/tests.rs b/substrate/frame/delegated-staking/src/tests.rs index bc8bc2dccc3c..2c965e18b1b3 100644 --- a/substrate/frame/delegated-staking/src/tests.rs +++ b/substrate/frame/delegated-staking/src/tests.rs @@ -334,6 +334,36 @@ fn apply_pending_slash() { }); } +#[test] +fn allow_full_amount_to_be_delegated() { + ExtBuilder::default().build_and_execute(|| { + let agent: AccountId = 200; + let reward_acc: AccountId = 201; + let delegator: AccountId = 300; + + // set intention to accept delegation. + fund(&agent, 1000); + assert_ok!(DelegatedStaking::register_agent(RawOrigin::Signed(agent).into(), reward_acc)); + + // delegate to this account + fund(&delegator, 1000); + assert_ok!(DelegatedStaking::delegate_to_agent( + RawOrigin::Signed(delegator).into(), + agent, + 1000 + )); + + // verify + assert!(DelegatedStaking::is_agent(&agent)); + assert_eq!(DelegatedStaking::stakeable_balance(Agent::from(agent)), 1000); + assert_eq!( + Balances::balance_on_hold(&HoldReason::StakingDelegation.into(), &delegator), + 1000 + ); + assert_eq!(DelegatedStaking::held_balance_of(Delegator::from(delegator)), 1000); + }); +} + /// Integration tests with pallet-staking. mod staking_integration { use super::*; @@ -625,32 +655,42 @@ mod staking_integration { #[test] fn migration_works() { ExtBuilder::default().build_and_execute(|| { + // initial era + start_era(1); + // add a nominator let staked_amount = 4000; let agent_amount = 5000; - fund(&200, agent_amount); + let agent = 200; + fund(&agent, agent_amount); assert_ok!(Staking::bond( - RuntimeOrigin::signed(200), + RuntimeOrigin::signed(agent), staked_amount, RewardDestination::Account(201) )); - assert_ok!(Staking::nominate(RuntimeOrigin::signed(200), vec![GENESIS_VALIDATOR],)); - let init_stake = Staking::stake(&200).unwrap(); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(agent), vec![GENESIS_VALIDATOR],)); + let init_stake = Staking::stake(&agent).unwrap(); // scenario: 200 is a pool account, and the stake comes from its 4 delegators (300..304) // in equal parts. lets try to migrate this nominator into delegate based stake. // all balance currently is in 200 - assert_eq!(Balances::free_balance(200), agent_amount); + assert_eq!(Balances::free_balance(agent), agent_amount); // to migrate, nominator needs to set an account as a proxy delegator where staked funds // will be moved and delegated back to this old nominator account. This should be funded // with at least ED. let proxy_delegator = - DelegatedStaking::generate_proxy_delegator(Agent::from(200)).get(); + DelegatedStaking::generate_proxy_delegator(Agent::from(agent)).get(); - assert_ok!(DelegatedStaking::migrate_to_agent(RawOrigin::Signed(200).into(), 201)); + assert_ok!(DelegatedStaking::migrate_to_agent(RawOrigin::Signed(agent).into(), 201)); + // after migration, funds are moved to proxy delegator, still a provider exists. + assert_eq!(System::providers(&agent), 1); + assert_eq!(Balances::free_balance(agent), 0); + // proxy delegator has one provider as well with no free balance. + assert_eq!(System::providers(&proxy_delegator), 1); + assert_eq!(Balances::free_balance(proxy_delegator), 0); // verify all went well let mut expected_proxy_delegated_amount = agent_amount; @@ -659,12 +699,12 @@ mod staking_integration { expected_proxy_delegated_amount ); // stake amount is transferred from delegate to proxy delegator account. - assert_eq!(Balances::free_balance(200), 0); - assert_eq!(Staking::stake(&200).unwrap(), init_stake); - assert_eq!(get_agent_ledger(&200).ledger.effective_balance(), agent_amount); - assert_eq!(get_agent_ledger(&200).available_to_bond(), 0); + assert_eq!(Balances::free_balance(agent), 0); + assert_eq!(Staking::stake(&agent).unwrap(), init_stake); + assert_eq!(get_agent_ledger(&agent).ledger.effective_balance(), agent_amount); + assert_eq!(get_agent_ledger(&agent).available_to_bond(), 0); assert_eq!( - get_agent_ledger(&200).ledger.unclaimed_withdrawals, + get_agent_ledger(&agent).ledger.unclaimed_withdrawals, agent_amount - staked_amount ); @@ -672,14 +712,15 @@ mod staking_integration { let delegator_share = agent_amount / 4; for delegator in 300..304 { assert_eq!(Balances::free_balance(delegator), 0); - // fund them with ED - fund(&delegator, ExistentialDeposit::get()); - // migrate 1/4th amount into each delegator + assert_eq!(System::providers(&delegator), 0); + + // No pre-balance needed to migrate delegator. assert_ok!(DelegatedStaking::migrate_delegation( - RawOrigin::Signed(200).into(), + RawOrigin::Signed(agent).into(), delegator, delegator_share )); + assert_eq!(System::providers(&delegator), 1); assert_eq!( Balances::balance_on_hold(&HoldReason::StakingDelegation.into(), &delegator), delegator_share @@ -694,20 +735,123 @@ mod staking_integration { ); // delegate stake is unchanged. - assert_eq!(Staking::stake(&200).unwrap(), init_stake); - assert_eq!(get_agent_ledger(&200).ledger.effective_balance(), agent_amount); - assert_eq!(get_agent_ledger(&200).available_to_bond(), 0); + assert_eq!(Staking::stake(&agent).unwrap(), init_stake); + assert_eq!(get_agent_ledger(&agent).ledger.effective_balance(), agent_amount); + assert_eq!(get_agent_ledger(&agent).available_to_bond(), 0); assert_eq!( - get_agent_ledger(&200).ledger.unclaimed_withdrawals, + get_agent_ledger(&agent).ledger.unclaimed_withdrawals, agent_amount - staked_amount ); } // cannot use migrate delegator anymore assert_noop!( - DelegatedStaking::migrate_delegation(RawOrigin::Signed(200).into(), 305, 1), + DelegatedStaking::migrate_delegation(RawOrigin::Signed(agent).into(), 305, 1), Error::::NotEnoughFunds ); + + // no provider left on proxy delegator since all funds are migrated + assert_eq!(System::providers(&proxy_delegator), 0); + + // withdraw all delegations from delegators + assert_ok!(Staking::chill(RuntimeOrigin::signed(agent))); + assert_ok!(Staking::unbond(RawOrigin::Signed(agent).into(), staked_amount)); + start_era(4); + assert_ok!(Staking::withdraw_unbonded(RawOrigin::Signed(agent).into(), 0)); + for delegator in 300..304 { + assert_ok!(DelegatedStaking::release_delegation( + RawOrigin::Signed(agent).into(), + delegator, + delegator_share, + 0 + )); + // delegator is cleaned up from storage. + assert!(!Delegators::::contains_key(delegator)); + // has free balance now + assert_eq!(Balances::free_balance(delegator), delegator_share); + // and only one provider as delegator_share > ED + assert_eq!(System::providers(&delegator), 1); + } + + // Agent can be removed now. + assert_ok!(DelegatedStaking::remove_agent(RawOrigin::Signed(agent).into())); + // agent is correctly removed. + assert!(!Agents::::contains_key(agent)); + // and no provider left. + assert_eq!(System::providers(&agent), 0); + }); + } + + #[test] + fn accounts_are_cleaned_up() { + ExtBuilder::default().build_and_execute(|| { + let agent: AccountId = 200; + let reward_acc: AccountId = 201; + let delegator: AccountId = 300; + + // set intention to accept delegation. + fund(&agent, 1000); + + // Agent is provided since it has balance > ED. + assert_eq!(System::providers(&agent), 1); + assert_ok!(DelegatedStaking::register_agent( + RawOrigin::Signed(agent).into(), + reward_acc + )); + // becoming an agent adds another provider. + assert_eq!(System::providers(&agent), 2); + + // delegate to this account + fund(&delegator, 1000); + // account has one provider since its funded. + assert_eq!(System::providers(&delegator), 1); + assert_ok!(DelegatedStaking::delegate_to_agent( + RawOrigin::Signed(delegator).into(), + agent, + 500 + )); + // delegator has an extra provider now. + assert_eq!(System::providers(&delegator), 2); + // all 1000 tokens including ED can be held. + assert_ok!(DelegatedStaking::delegate_to_agent( + RawOrigin::Signed(delegator).into(), + agent, + 500 + )); + // free balance dropping below ED will reduce a provider, but it still has one provider + // left. + assert_eq!(System::providers(&delegator), 1); + + // withdraw all delegation + assert_ok!(Staking::unbond(RawOrigin::Signed(agent).into(), 1000)); + start_era(4); + assert_ok!(Staking::withdraw_unbonded(RawOrigin::Signed(agent).into(), 0)); + + // Since delegations are still left, agents cannot be removed yet from storage. + assert_noop!( + DelegatedStaking::remove_agent(RawOrigin::Signed(agent).into()), + Error::::NotAllowed + ); + + assert_ok!(DelegatedStaking::release_delegation( + RawOrigin::Signed(agent).into(), + delegator, + 1000, + 0 + )); + + // now agents can be removed. + assert_ok!(DelegatedStaking::remove_agent(RawOrigin::Signed(agent).into())); + + // agent and delegator provider is decremented. + assert_eq!(System::providers(&delegator), 1); + assert_eq!(System::providers(&agent), 1); + + // if we transfer all funds, providers are removed. + assert_ok!(Balances::transfer_all(RawOrigin::Signed(delegator).into(), 1337, false)); + assert_ok!(Balances::transfer_all(RawOrigin::Signed(agent).into(), 1337, false)); + assert_eq!(System::providers(&delegator), 0); + assert_eq!(System::providers(&agent), 0); }); } } diff --git a/substrate/frame/delegated-staking/src/types.rs b/substrate/frame/delegated-staking/src/types.rs index aff282774c3c..a78aa3f55906 100644 --- a/substrate/frame/delegated-staking/src/types.rs +++ b/substrate/frame/delegated-staking/src/types.rs @@ -64,15 +64,25 @@ impl Delegation { ) } - /// Save self to storage. If the delegation amount is zero, remove the delegation. - pub(crate) fn update_or_kill(self, key: &T::AccountId) { - // Clean up if no delegation left. - if self.amount == Zero::zero() { - >::remove(key); - return + /// Save self to storage. + /// + /// If the delegation amount is zero, remove the delegation. Also adds and removes provider + /// reference as needed. + pub(crate) fn update(self, key: &T::AccountId) { + if >::contains_key(key) { + // Clean up if no delegation left. + if self.amount == Zero::zero() { + >::remove(key); + // Remove provider if no delegation left. + let _ = frame_system::Pallet::::dec_providers(key).defensive(); + return + } + } else { + // this is a new delegation. Provide for this account. + frame_system::Pallet::::inc_providers(key); } - >::insert(key, self) + >::insert(key, self); } } @@ -118,10 +128,24 @@ impl AgentLedger { } /// Save self to storage with the given key. + /// + /// Increments provider count if this is a new agent. pub(crate) fn update(self, key: &T::AccountId) { + if !>::contains_key(key) { + // This is a new agent. Provide for this account. + frame_system::Pallet::::inc_providers(key); + } >::insert(key, self) } + /// Remove self from storage. + pub(crate) fn remove(key: &T::AccountId) { + debug_assert!(>::contains_key(key), "Agent should exist in storage"); + >::remove(key); + // Remove provider reference. + let _ = frame_system::Pallet::::dec_providers(key).defensive(); + } + /// Effective total balance of the `Agent`. /// /// This takes into account any slashes reported to `Agent` but unapplied. diff --git a/substrate/frame/nomination-pools/runtime-api/src/lib.rs b/substrate/frame/nomination-pools/runtime-api/src/lib.rs index 67627e0acb13..d81ad1dd4954 100644 --- a/substrate/frame/nomination-pools/runtime-api/src/lib.rs +++ b/substrate/frame/nomination-pools/runtime-api/src/lib.rs @@ -63,5 +63,11 @@ sp_api::decl_runtime_apis! { /// [`migrate_delegation`](pallet_nomination_pools::Call::migrate_delegation) /// to migrate the funds of the pool member. fn member_needs_delegate_migration(member: AccountId) -> bool; + + /// Returns the total contribution of a pool member including any balance that is unbonding. + fn member_total_balance(who: AccountId) -> Balance; + + /// Total balance contributed to the pool. + fn pool_balance(pool_id: PoolId) -> Balance; } } diff --git a/substrate/frame/nomination-pools/src/lib.rs b/substrate/frame/nomination-pools/src/lib.rs index 9108060ccb2a..44e3463dc9f2 100644 --- a/substrate/frame/nomination-pools/src/lib.rs +++ b/substrate/frame/nomination-pools/src/lib.rs @@ -2010,6 +2010,8 @@ pub mod pallet { pool_id: PoolId, ) -> DispatchResult { let who = ensure_signed(origin)?; + // ensure pool is not in an un-migrated state. + ensure!(!Self::api_pool_needs_delegate_migration(pool_id), Error::::NotMigrated); ensure!(amount >= MinJoinBond::::get(), Error::::MinimumBondNotMet); // If a member already exists that means they already belong to a pool @@ -2072,6 +2074,13 @@ pub mod pallet { )] pub fn bond_extra(origin: OriginFor, extra: BondExtra>) -> DispatchResult { let who = ensure_signed(origin)?; + + // ensure who is not in an un-migrated state. + ensure!( + !Self::api_member_needs_delegate_migration(who.clone()), + Error::::NotMigrated + ); + Self::do_bond_extra(who.clone(), who, extra) } @@ -2087,6 +2096,12 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::claim_payout())] pub fn claim_payout(origin: OriginFor) -> DispatchResult { let signer = ensure_signed(origin)?; + // ensure signer is not in an un-migrated state. + ensure!( + !Self::api_member_needs_delegate_migration(signer.clone()), + Error::::NotMigrated + ); + Self::do_claim_payout(signer.clone(), signer) } @@ -2130,6 +2145,12 @@ pub mod pallet { ) -> DispatchResult { let who = ensure_signed(origin)?; let member_account = T::Lookup::lookup(member_account)?; + // ensure member is not in an un-migrated state. + ensure!( + !Self::api_member_needs_delegate_migration(member_account.clone()), + Error::::NotMigrated + ); + let (mut member, mut bonded_pool, mut reward_pool) = Self::get_member_with_pools(&member_account)?; @@ -2213,6 +2234,9 @@ pub mod pallet { num_slashing_spans: u32, ) -> DispatchResult { let _ = ensure_signed(origin)?; + // ensure pool is not in an un-migrated state. + ensure!(!Self::api_pool_needs_delegate_migration(pool_id), Error::::NotMigrated); + let pool = BondedPool::::get(pool_id).ok_or(Error::::PoolNotFound)?; // For now we only allow a pool to withdraw unbonded if its not destroying. If the pool @@ -2259,6 +2283,12 @@ pub mod pallet { ) -> DispatchResultWithPostInfo { let caller = ensure_signed(origin)?; let member_account = T::Lookup::lookup(member_account)?; + // ensure member is not in an un-migrated state. + ensure!( + !Self::api_member_needs_delegate_migration(member_account.clone()), + Error::::NotMigrated + ); + let mut member = PoolMembers::::get(&member_account).ok_or(Error::::PoolMemberNotFound)?; let current_era = T::StakeAdapter::current_era(); @@ -2493,6 +2523,8 @@ pub mod pallet { ) -> DispatchResult { let who = ensure_signed(origin)?; let bonded_pool = BondedPool::::get(pool_id).ok_or(Error::::PoolNotFound)?; + // ensure pool is not in an un-migrated state. + ensure!(!Self::api_pool_needs_delegate_migration(pool_id), Error::::NotMigrated); ensure!(bonded_pool.can_nominate(&who), Error::::NotNominator); let depositor_points = PoolMembers::::get(&bonded_pool.roles.depositor) @@ -2527,6 +2559,8 @@ pub mod pallet { let who = ensure_signed(origin)?; let mut bonded_pool = BondedPool::::get(pool_id).ok_or(Error::::PoolNotFound)?; ensure!(bonded_pool.state != PoolState::Destroying, Error::::CanNotChangeState); + // ensure pool is not in an un-migrated state. + ensure!(!Self::api_pool_needs_delegate_migration(pool_id), Error::::NotMigrated); if bonded_pool.can_toggle_state(&who) { bonded_pool.set_state(state); @@ -2562,6 +2596,8 @@ pub mod pallet { .can_set_metadata(&who), Error::::DoesNotHavePermission ); + // ensure pool is not in an un-migrated state. + ensure!(!Self::api_pool_needs_delegate_migration(pool_id), Error::::NotMigrated); Metadata::::mutate(pool_id, |pool_meta| *pool_meta = metadata); @@ -2638,6 +2674,9 @@ pub mod pallet { }, }; + // ensure pool is not in an un-migrated state. + ensure!(!Self::api_pool_needs_delegate_migration(pool_id), Error::::NotMigrated); + match new_root { ConfigOp::Noop => (), ConfigOp::Remove => bonded_pool.roles.root = None, @@ -2684,8 +2723,9 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::chill())] pub fn chill(origin: OriginFor, pool_id: PoolId) -> DispatchResult { let who = ensure_signed(origin)?; - let bonded_pool = BondedPool::::get(pool_id).ok_or(Error::::PoolNotFound)?; + // ensure pool is not in an un-migrated state. + ensure!(!Self::api_pool_needs_delegate_migration(pool_id), Error::::NotMigrated); let depositor_points = PoolMembers::::get(&bonded_pool.roles.depositor) .ok_or(Error::::PoolMemberNotFound)? @@ -2720,7 +2760,14 @@ pub mod pallet { extra: BondExtra>, ) -> DispatchResult { let who = ensure_signed(origin)?; - Self::do_bond_extra(who, T::Lookup::lookup(member)?, extra) + let member_account = T::Lookup::lookup(member)?; + // ensure member is not in an un-migrated state. + ensure!( + !Self::api_member_needs_delegate_migration(member_account.clone()), + Error::::NotMigrated + ); + + Self::do_bond_extra(who, member_account, extra) } /// Allows a pool member to set a claim permission to allow or disallow permissionless @@ -2737,9 +2784,14 @@ pub mod pallet { permission: ClaimPermission, ) -> DispatchResult { let who = ensure_signed(origin)?; - ensure!(PoolMembers::::contains_key(&who), Error::::PoolMemberNotFound); + // ensure member is not in an un-migrated state. + ensure!( + !Self::api_member_needs_delegate_migration(who.clone()), + Error::::NotMigrated + ); + ClaimPermissions::::mutate(who, |source| { *source = permission; }); @@ -2755,6 +2807,12 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::claim_payout())] pub fn claim_payout_other(origin: OriginFor, other: T::AccountId) -> DispatchResult { let signer = ensure_signed(origin)?; + // ensure member is not in an un-migrated state. + ensure!( + !Self::api_member_needs_delegate_migration(other.clone()), + Error::::NotMigrated + ); + Self::do_claim_payout(signer, other) } @@ -2773,6 +2831,9 @@ pub mod pallet { ) -> DispatchResult { let who = ensure_signed(origin)?; let mut bonded_pool = BondedPool::::get(pool_id).ok_or(Error::::PoolNotFound)?; + // ensure pool is not in an un-migrated state. + ensure!(!Self::api_pool_needs_delegate_migration(pool_id), Error::::NotMigrated); + ensure!(bonded_pool.can_manage_commission(&who), Error::::DoesNotHavePermission); let mut reward_pool = RewardPools::::get(pool_id) @@ -2809,6 +2870,9 @@ pub mod pallet { ) -> DispatchResult { let who = ensure_signed(origin)?; let mut bonded_pool = BondedPool::::get(pool_id).ok_or(Error::::PoolNotFound)?; + // ensure pool is not in an un-migrated state. + ensure!(!Self::api_pool_needs_delegate_migration(pool_id), Error::::NotMigrated); + ensure!(bonded_pool.can_manage_commission(&who), Error::::DoesNotHavePermission); bonded_pool.commission.try_update_max(pool_id, max_commission)?; @@ -2831,6 +2895,8 @@ pub mod pallet { ) -> DispatchResult { let who = ensure_signed(origin)?; let mut bonded_pool = BondedPool::::get(pool_id).ok_or(Error::::PoolNotFound)?; + // ensure pool is not in an un-migrated state. + ensure!(!Self::api_pool_needs_delegate_migration(pool_id), Error::::NotMigrated); ensure!(bonded_pool.can_manage_commission(&who), Error::::DoesNotHavePermission); bonded_pool.commission.try_update_change_rate(change_rate)?; @@ -2852,6 +2918,9 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::claim_commission())] pub fn claim_commission(origin: OriginFor, pool_id: PoolId) -> DispatchResult { let who = ensure_signed(origin)?; + // ensure pool is not in an un-migrated state. + ensure!(!Self::api_pool_needs_delegate_migration(pool_id), Error::::NotMigrated); + Self::do_claim_commission(who, pool_id) } @@ -2866,6 +2935,9 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::adjust_pool_deposit())] pub fn adjust_pool_deposit(origin: OriginFor, pool_id: PoolId) -> DispatchResult { let who = ensure_signed(origin)?; + // ensure pool is not in an un-migrated state. + ensure!(!Self::api_pool_needs_delegate_migration(pool_id), Error::::NotMigrated); + Self::do_adjust_pool_deposit(who, pool_id) } @@ -2882,6 +2954,8 @@ pub mod pallet { ) -> DispatchResult { let who = ensure_signed(origin)?; let mut bonded_pool = BondedPool::::get(pool_id).ok_or(Error::::PoolNotFound)?; + // ensure pool is not in an un-migrated state. + ensure!(!Self::api_pool_needs_delegate_migration(pool_id), Error::::NotMigrated); ensure!(bonded_pool.can_manage_commission(&who), Error::::DoesNotHavePermission); bonded_pool.commission.claim_permission = permission.clone(); @@ -2956,9 +3030,12 @@ pub mod pallet { ); let pool_contribution = member.total_balance(); - ensure!(pool_contribution >= MinJoinBond::::get(), Error::::MinimumBondNotMet); - // the member must have some contribution to be migrated. - ensure!(pool_contribution > Zero::zero(), Error::::AlreadyMigrated); + // ensure the pool contribution is greater than the existential deposit otherwise we + // cannot transfer funds to member account. + ensure!( + pool_contribution >= T::Currency::minimum_balance(), + Error::::MinimumBondNotMet + ); let delegation = T::StakeAdapter::member_delegation_balance(Member::from(member_account.clone())); @@ -3459,6 +3536,7 @@ impl Pallet { fn do_adjust_pool_deposit(who: T::AccountId, pool: PoolId) -> DispatchResult { let bonded_pool = BondedPool::::get(pool).ok_or(Error::::PoolNotFound)?; + let reward_acc = &bonded_pool.reward_account(); let pre_frozen_balance = T::Currency::balance_frozen(&FreezeReason::PoolMinBalance.into(), reward_acc); @@ -3880,7 +3958,13 @@ impl Pallet { return false } + // if pool does not exist, return false. + if !BondedPools::::contains_key(pool_id) { + return false + } + let pool_account = Self::generate_bonded_account(pool_id); + // true if pool is still not migrated to `DelegateStake`. T::StakeAdapter::pool_strategy(Pool::from(pool_account)) != adapter::StakeStrategyType::Delegate @@ -3914,6 +3998,22 @@ impl Pallet { }) .unwrap_or_default() } + + /// Contribution of the member in the pool. + /// + /// Includes balance that is unbonded from staking but not claimed yet from the pool, therefore + /// this balance can be higher than the staked funds. + pub fn api_member_total_balance(who: T::AccountId) -> BalanceOf { + PoolMembers::::get(who.clone()) + .map(|m| m.total_balance()) + .unwrap_or_default() + } + + /// Total balance contributed to the pool. + pub fn api_pool_balance(pool_id: PoolId) -> BalanceOf { + T::StakeAdapter::total_balance(Pool::from(Self::generate_bonded_account(pool_id))) + .unwrap_or_default() + } } impl sp_staking::OnStakingUpdate> for Pallet { diff --git a/substrate/frame/nomination-pools/test-delegate-stake/src/lib.rs b/substrate/frame/nomination-pools/test-delegate-stake/src/lib.rs index a50a6b73f5fc..7fee2a0bdb23 100644 --- a/substrate/frame/nomination-pools/test-delegate-stake/src/lib.rs +++ b/substrate/frame/nomination-pools/test-delegate-stake/src/lib.rs @@ -25,16 +25,16 @@ use frame_support::{ }; use mock::*; use pallet_nomination_pools::{ - BondExtra, BondedPools, Error as PoolsError, Event as PoolsEvent, LastPoolId, PoolMember, - PoolMembers, PoolState, + BondExtra, BondedPools, CommissionChangeRate, ConfigOp, Error as PoolsError, + Event as PoolsEvent, LastPoolId, PoolMember, PoolMembers, PoolState, }; use pallet_staking::{ CurrentEra, Error as StakingError, Event as StakingEvent, Payee, RewardDestination, }; -use pallet_delegated_staking::{Error as DelegatedStakingError, Event as DelegatedStakingEvent}; +use pallet_delegated_staking::Event as DelegatedStakingEvent; -use sp_runtime::{bounded_btree_map, traits::Zero}; +use sp_runtime::{bounded_btree_map, traits::Zero, Perbill}; use sp_staking::Agent; #[test] @@ -999,7 +999,6 @@ fn pool_migration_e2e() { LegacyAdapter::set(false); // cannot migrate the member delegation unless pool is migrated first. - assert!(!Pools::api_member_needs_delegate_migration(20)); assert_noop!( Pools::migrate_delegation(RuntimeOrigin::signed(10), 20), PoolsError::::NotMigrated @@ -1031,13 +1030,17 @@ fn pool_migration_e2e() { // move to era 5 when 20 can withdraw unbonded funds. CurrentEra::::set(Some(5)); - // Unbond works even without claiming delegation. Lets unbond 22. - assert_ok!(Pools::unbond(RuntimeOrigin::signed(22), 22, 5)); + + // Cannot unbond without claiming delegation. Lets unbond 22. + assert_noop!( + Pools::unbond(RuntimeOrigin::signed(22), 22, 5), + PoolsError::::NotMigrated + ); // withdraw fails for 20 before claiming delegation assert_noop!( Pools::withdraw_unbonded(RuntimeOrigin::signed(20), 20, 10), - DelegatedStakingError::::NotDelegator + PoolsError::::NotMigrated ); let pre_claim_balance_20 = Balances::total_balance(&20); @@ -1060,17 +1063,11 @@ fn pool_migration_e2e() { assert_eq!( staking_events_since_last_call(), - vec![ - StakingEvent::Unbonded { stash: POOL1_BONDED, amount: 5 }, - StakingEvent::Withdrawn { stash: POOL1_BONDED, amount: 5 } - ] + vec![StakingEvent::Withdrawn { stash: POOL1_BONDED, amount: 5 }] ); assert_eq!( pool_events_since_last_call(), - vec![ - PoolsEvent::Unbonded { member: 22, pool_id: 1, balance: 5, points: 5, era: 8 }, - PoolsEvent::Withdrawn { member: 20, pool_id: 1, balance: 5, points: 5 }, - ] + vec![PoolsEvent::Withdrawn { member: 20, pool_id: 1, balance: 5, points: 5 },] ); assert_eq!( delegated_staking_events_since_last_call(), @@ -1113,8 +1110,9 @@ fn pool_migration_e2e() { assert_eq!(Balances::total_balance(&21), pre_migrate_balance_21 + 10); // MIGRATE 22 - let pre_migrate_balance_22 = Balances::total_balance(&22); assert_eq!(Balances::total_balance_on_hold(&22), 0); + // make balance of 22 as 0. + let _ = Balances::make_free_balance_be(&22, 0); // migrate delegation for 22. assert!(Pools::api_member_needs_delegate_migration(22)); @@ -1128,17 +1126,20 @@ fn pool_migration_e2e() { ); // tokens moved to 22's account and held there. - assert_eq!(Balances::total_balance(&22), pre_migrate_balance_22 + 10); + assert_eq!(Balances::total_balance(&22), 10); assert_eq!(Balances::total_balance_on_hold(&22), 10); - // withdraw fails since 22 only unbonds at era 8. + // unbond 22 should work now + assert_ok!(Pools::unbond(RuntimeOrigin::signed(22), 22, 5)); + + // withdraw fails since 22 only unbonds after era 9. assert_noop!( Pools::withdraw_unbonded(RuntimeOrigin::signed(22), 22, 5), PoolsError::::CannotWithdrawAny ); // go to era when 22 can unbond - CurrentEra::::set(Some(10)); + CurrentEra::::set(Some(9)); // withdraw works now assert_ok!(Pools::withdraw_unbonded(RuntimeOrigin::signed(22), 22, 10)); @@ -1151,6 +1152,7 @@ fn pool_migration_e2e() { staking_events_since_last_call(), vec![ StakingEvent::Withdrawn { stash: POOL1_BONDED, amount: 10 }, + StakingEvent::Unbonded { stash: POOL1_BONDED, amount: 5 }, StakingEvent::Withdrawn { stash: POOL1_BONDED, amount: 5 } ] ); @@ -1161,6 +1163,7 @@ fn pool_migration_e2e() { PoolsEvent::Withdrawn { member: 21, pool_id: 1, balance: 10, points: 10 }, // 21 was fully unbonding and removed from pool. PoolsEvent::MemberRemoved { member: 21, pool_id: 1, released_balance: 0 }, + PoolsEvent::Unbonded { member: 22, pool_id: 1, balance: 5, points: 5, era: 9 }, PoolsEvent::Withdrawn { member: 22, pool_id: 1, balance: 5, points: 5 }, ] ); @@ -1184,6 +1187,183 @@ fn pool_migration_e2e() { }) } +#[test] +fn disable_pool_operations_on_non_migrated() { + new_test_ext().execute_with(|| { + LegacyAdapter::set(true); + assert_eq!(Balances::minimum_balance(), 5); + assert_eq!(Staking::current_era(), None); + + // create the pool with TransferStake strategy. + assert_ok!(Pools::create(RuntimeOrigin::signed(10), 50, 10, 10, 10)); + assert_eq!(LastPoolId::::get(), 1); + + // have the pool nominate. + assert_ok!(Pools::nominate(RuntimeOrigin::signed(10), 1, vec![1, 2, 3])); + + assert_eq!( + staking_events_since_last_call(), + vec![StakingEvent::Bonded { stash: POOL1_BONDED, amount: 50 }] + ); + assert_eq!( + pool_events_since_last_call(), + vec![ + PoolsEvent::Created { depositor: 10, pool_id: 1 }, + PoolsEvent::Bonded { member: 10, pool_id: 1, bonded: 50, joined: true }, + ] + ); + + let pre_20 = Balances::free_balance(20); + assert_ok!(Pools::join(RuntimeOrigin::signed(20), 10, 1)); + + // verify members balance is moved to pool. + assert_eq!(Balances::free_balance(20), pre_20 - 10); + + assert_eq!( + staking_events_since_last_call(), + vec![StakingEvent::Bonded { stash: POOL1_BONDED, amount: 10 },] + ); + assert_eq!( + pool_events_since_last_call(), + vec![PoolsEvent::Bonded { member: 20, pool_id: 1, bonded: 10, joined: true },] + ); + + // we reset the adapter to `DelegateStake`. + LegacyAdapter::set(false); + + // pool is pending migration. + assert!(Pools::api_pool_needs_delegate_migration(1)); + + // ensure pool mutation is not allowed until pool is migrated. + assert_noop!( + Pools::join(RuntimeOrigin::signed(21), 10, 1), + PoolsError::::NotMigrated + ); + assert_noop!( + Pools::pool_withdraw_unbonded(RuntimeOrigin::signed(10), 1, 0), + PoolsError::::NotMigrated + ); + assert_noop!( + Pools::nominate(RuntimeOrigin::signed(10), 1, vec![1, 2, 3]), + PoolsError::::NotMigrated + ); + assert_noop!( + Pools::set_state(RuntimeOrigin::signed(10), 1, PoolState::Blocked), + PoolsError::::NotMigrated + ); + assert_noop!( + Pools::set_metadata(RuntimeOrigin::signed(10), 1, vec![1, 1]), + PoolsError::::NotMigrated + ); + assert_noop!( + Pools::update_roles( + RuntimeOrigin::signed(10), + 1, + ConfigOp::Set(5), + ConfigOp::Set(6), + ConfigOp::Set(7) + ), + PoolsError::::NotMigrated + ); + assert_noop!( + Pools::chill(RuntimeOrigin::signed(10), 1), + PoolsError::::NotMigrated + ); + assert_noop!( + Pools::set_commission(RuntimeOrigin::signed(10), 1, None), + PoolsError::::NotMigrated + ); + assert_noop!( + Pools::set_commission_max(RuntimeOrigin::signed(10), 1, Zero::zero()), + PoolsError::::NotMigrated + ); + assert_noop!( + Pools::set_commission_change_rate( + RuntimeOrigin::signed(10), + 1, + CommissionChangeRate { max_increase: Perbill::from_percent(1), min_delay: 2_u64 } + ), + PoolsError::::NotMigrated + ); + assert_noop!( + Pools::claim_commission(RuntimeOrigin::signed(10), 1), + PoolsError::::NotMigrated + ); + assert_noop!( + Pools::adjust_pool_deposit(RuntimeOrigin::signed(10), 1), + PoolsError::::NotMigrated + ); + assert_noop!( + Pools::set_commission_claim_permission(RuntimeOrigin::signed(10), 1, None), + PoolsError::::NotMigrated + ); + + // migrate the pool. + assert_ok!(Pools::migrate_pool_to_delegate_stake(RuntimeOrigin::signed(10), 1)); + assert_eq!( + delegated_staking_events_since_last_call(), + vec![DelegatedStakingEvent::Delegated { + agent: POOL1_BONDED, + delegator: DelegatedStaking::generate_proxy_delegator(Agent::from(POOL1_BONDED)) + .get(), + amount: 50 + 10 + },] + ); + + // member is pending migration. + assert!(Pools::api_member_needs_delegate_migration(20)); + + // ensure member mutation is not allowed until member's delegation is migrated. + assert_noop!( + Pools::bond_extra(RuntimeOrigin::signed(20), BondExtra::FreeBalance(5)), + PoolsError::::NotMigrated + ); + assert_noop!( + Pools::bond_extra_other(RuntimeOrigin::signed(10), 20, BondExtra::Rewards), + PoolsError::::NotMigrated + ); + assert_noop!( + Pools::claim_payout(RuntimeOrigin::signed(20)), + PoolsError::::NotMigrated + ); + assert_noop!( + Pools::unbond(RuntimeOrigin::signed(20), 20, 5), + PoolsError::::NotMigrated + ); + assert_noop!( + Pools::withdraw_unbonded(RuntimeOrigin::signed(20), 20, 0), + PoolsError::::NotMigrated + ); + + // migrate 20 + assert_ok!(Pools::migrate_delegation(RuntimeOrigin::signed(10), 20)); + // now `bond_extra` for 20 works. + assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(20), BondExtra::FreeBalance(5))); + + assert_eq!( + staking_events_since_last_call(), + vec![StakingEvent::Bonded { stash: POOL1_BONDED, amount: 5 },] + ); + + assert_eq!( + pool_events_since_last_call(), + vec![PoolsEvent::Bonded { member: 20, pool_id: 1, bonded: 5, joined: false },] + ); + + assert_eq!( + delegated_staking_events_since_last_call(), + vec![ + DelegatedStakingEvent::MigratedDelegation { + agent: POOL1_BONDED, + delegator: 20, + amount: 10 + }, + DelegatedStakingEvent::Delegated { agent: POOL1_BONDED, delegator: 20, amount: 5 }, + ] + ); + }) +} + #[test] fn pool_no_dangling_delegation() { new_test_ext().execute_with(|| { @@ -1214,6 +1394,7 @@ fn pool_no_dangling_delegation() { delegated_staking_events_since_last_call(), vec![DelegatedStakingEvent::Delegated { agent: POOL1_BONDED, + delegator: alice, amount: 40 },]