From d9722f19fbdebcf92a9827f6cfda9c2d7de94263 Mon Sep 17 00:00:00 2001 From: Ankan <10196091+Ank4n@users.noreply.github.com> Date: Mon, 3 Jun 2024 09:27:00 +0200 Subject: [PATCH] [Pools] Refactors and runtime apis for DelegateStake (#4537) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Runtime Apis Introduces the following runtime apis to facilitate dapps and wallets in integrating with the `DelegateStake` functionalities of the pools (related: https://github.com/paritytech/polkadot-sdk/pull/3905). These apis are meant to support pool and member migration, as well as lazy application of pending slashes of pool members. ```rust fn pool_pending_slash(pool_id: PoolId) -> Balance; fn member_pending_slash(member: AccountId) -> Balance; fn pool_needs_delegate_migration(pool_id: PoolId) -> bool; fn member_needs_delegate_migration(member: AccountId) -> bool; ``` ## Refactors - Introduces newtypes for `Agent`, `Delegator`, `Pool` and `[Pool]Member`. And refactors `StakeAdapter` and `DelegationInterface` to accept the above types. This will help make these apis typesafe against using wrong account type. - Fixing `DelegationInterface` apis to return optional (instead of default value if key does not exist). - Rename struct `Agent` that wraps `AgentLedger` to `AgentOuterLedger` which is clearer (naming wise) and different from the newtype `Agent`. - Cleaning up new Pool events (related to `Delegation` feature of pool). --------- Signed-off-by: Matteo Muraca Signed-off-by: Alexandru Gheorghe Signed-off-by: Andrei Sandu Signed-off-by: Adrian Catangiu Signed-off-by: Alexandru Vasile Signed-off-by: Oliver Tale-Yazdi Signed-off-by: divdeploy Signed-off-by: dependabot[bot] Signed-off-by: hongkuang Co-authored-by: Bastian Köcher Co-authored-by: gemini132 <164285545+gemini132@users.noreply.github.com> Co-authored-by: Matteo Muraca <56828990+muraca@users.noreply.github.com> Co-authored-by: Liam Aharon Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Co-authored-by: Alexandru Gheorghe <49718502+alexggh@users.noreply.github.com> Co-authored-by: Alessandro Siniscalchi Co-authored-by: Andrei Sandu <54316454+sandreim@users.noreply.github.com> Co-authored-by: Ross Bulat Co-authored-by: Serban Iorga Co-authored-by: s0me0ne-unkn0wn <48632512+s0me0ne-unkn0wn@users.noreply.github.com> Co-authored-by: Sam Johnson Co-authored-by: Adrian Catangiu Co-authored-by: Javier Viola <363911+pepoviola@users.noreply.github.com> Co-authored-by: Alexandru Vasile <60601340+lexnv@users.noreply.github.com> Co-authored-by: Niklas Adolfsson Co-authored-by: Dastan <88332432+dastansam@users.noreply.github.com> Co-authored-by: Clara van Staden Co-authored-by: Ron Co-authored-by: Vincent Geddes Co-authored-by: Svyatoslav Nikolsky Co-authored-by: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Co-authored-by: Dino Pačandi <3002868+Dinonard@users.noreply.github.com> Co-authored-by: Andrei Eres Co-authored-by: Alin Dima Co-authored-by: Andrei Sandu Co-authored-by: Oliver Tale-Yazdi Co-authored-by: Bastian Köcher Co-authored-by: Branislav Kontur Co-authored-by: Sebastian Kunert Co-authored-by: gupnik Co-authored-by: Vladimir Istyufeev Co-authored-by: Lulu Co-authored-by: Juan Girini Co-authored-by: Francisco Aguirre Co-authored-by: Dónal Murray Co-authored-by: Shawn Tabrizi Co-authored-by: Kutsal Kaan Bilgin Co-authored-by: Ermal Kaleci Co-authored-by: ordian Co-authored-by: divdeploy <166095818+divdeploy@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Co-authored-by: Squirrel Co-authored-by: HongKuang <166261675+HongKuang@users.noreply.github.com> Co-authored-by: Tsvetomir Dimitrov Co-authored-by: Egor_P Co-authored-by: Aaro Altonen <48052676+altonen@users.noreply.github.com> Co-authored-by: Dmitry Markin Co-authored-by: Alexandru Vasile Co-authored-by: Léa Narzis <78718413+lean-apple@users.noreply.github.com> Co-authored-by: Gonçalo Pestana Co-authored-by: georgepisaltu <52418509+georgepisaltu@users.noreply.github.com> Co-authored-by: command-bot <> Co-authored-by: PG Herveou Co-authored-by: jimwfs Co-authored-by: jimwfs <169986508+jimwfs@users.noreply.github.com> Co-authored-by: polka.dom --- polkadot/runtime/westend/src/lib.rs | 16 ++ prdoc/pr_4537.prdoc | 27 +++ substrate/bin/node/runtime/src/lib.rs | 16 ++ .../frame/delegated-staking/src/impls.rs | 75 +++--- substrate/frame/delegated-staking/src/lib.rs | 170 +++++++------ substrate/frame/delegated-staking/src/mock.rs | 14 +- .../frame/delegated-staking/src/tests.rs | 161 ++++++++---- .../frame/delegated-staking/src/types.rs | 19 +- .../benchmarking/src/inner.rs | 91 +++---- .../nomination-pools/runtime-api/src/lib.rs | 25 ++ .../frame/nomination-pools/src/adapter.rs | 207 ++++++++++------ substrate/frame/nomination-pools/src/lib.rs | 229 ++++++++++++------ .../frame/nomination-pools/src/migration.rs | 22 +- substrate/frame/nomination-pools/src/tests.rs | 6 + .../test-delegate-stake/src/lib.rs | 33 ++- .../test-delegate-stake/src/mock.rs | 35 +-- substrate/primitives/staking/src/lib.rs | 85 ++++--- 17 files changed, 800 insertions(+), 431 deletions(-) create mode 100644 prdoc/pr_4537.prdoc diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index 71ff1255907b4..bcdb00c763377 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -2287,6 +2287,22 @@ sp_api::impl_runtime_apis! { fn balance_to_points(pool_id: pallet_nomination_pools::PoolId, new_funds: Balance) -> Balance { NominationPools::api_balance_to_points(pool_id, new_funds) } + + fn pool_pending_slash(pool_id: pallet_nomination_pools::PoolId) -> Balance { + NominationPools::api_pool_pending_slash(pool_id) + } + + fn member_pending_slash(member: AccountId) -> Balance { + NominationPools::api_member_pending_slash(member) + } + + fn pool_needs_delegate_migration(pool_id: pallet_nomination_pools::PoolId) -> bool { + NominationPools::api_pool_needs_delegate_migration(pool_id) + } + + fn member_needs_delegate_migration(member: AccountId) -> bool { + NominationPools::api_member_needs_delegate_migration(member) + } } impl pallet_staking_runtime_api::StakingApi for Runtime { diff --git a/prdoc/pr_4537.prdoc b/prdoc/pr_4537.prdoc new file mode 100644 index 0000000000000..0148c95fb4e8b --- /dev/null +++ b/prdoc/pr_4537.prdoc @@ -0,0 +1,27 @@ +# 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: Runtime apis to help with delegate-stake based Nomination Pools. + +doc: + - audience: Runtime User + description: | + Introduces a new set of runtime apis to facilitate dapps and wallets to integrate with delegate-stake + functionalities of Nomination Pools. These apis support pool and member migration, as well as lazy application of + pending slashes of the pool members. + +crates: + - name: pallet-nomination-pools + bump: minor + - name: westend-runtime + bump: minor + - name: kitchensink-runtime + bump: minor + - name: pallet-delegated-staking + bump: minor + - name: sp-staking + bump: minor + - name: pallet-nomination-pools-benchmarking + 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 801abc28d3dda..8fb59a9d8474c 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -2774,6 +2774,22 @@ impl_runtime_apis! { fn balance_to_points(pool_id: pallet_nomination_pools::PoolId, new_funds: Balance) -> Balance { NominationPools::api_balance_to_points(pool_id, new_funds) } + + fn pool_pending_slash(pool_id: pallet_nomination_pools::PoolId) -> Balance { + NominationPools::api_pool_pending_slash(pool_id) + } + + fn member_pending_slash(member: AccountId) -> Balance { + NominationPools::api_member_pending_slash(member) + } + + fn pool_needs_delegate_migration(pool_id: pallet_nomination_pools::PoolId) -> bool { + NominationPools::api_pool_needs_delegate_migration(pool_id) + } + + fn member_needs_delegate_migration(member: AccountId) -> bool { + NominationPools::api_member_needs_delegate_migration(member) + } } 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 032f612064285..9f5649672d70e 100644 --- a/substrate/frame/delegated-staking/src/impls.rs +++ b/substrate/frame/delegated-staking/src/impls.rs @@ -19,46 +19,46 @@ //! Implementations of public traits, namely [`DelegationInterface`] and [`OnStakingUpdate`]. use super::*; -use sp_staking::{DelegationInterface, DelegationMigrator, OnStakingUpdate}; +use sp_staking::{Agent, DelegationInterface, DelegationMigrator, Delegator, OnStakingUpdate}; impl DelegationInterface for Pallet { type Balance = BalanceOf; type AccountId = T::AccountId; /// Effective balance of the `Agent` account. - fn agent_balance(who: &Self::AccountId) -> Self::Balance { - Agent::::get(who) - .map(|agent| agent.ledger.effective_balance()) - .unwrap_or_default() + fn agent_balance(agent: Agent) -> Option { + AgentLedgerOuter::::get(&agent.get()) + .map(|a| a.ledger.effective_balance()) + .ok() } - fn delegator_balance(delegator: &Self::AccountId) -> Self::Balance { - Delegation::::get(delegator).map(|d| d.amount).unwrap_or_default() + fn delegator_balance(delegator: Delegator) -> Option { + Delegation::::get(&delegator.get()).map(|d| d.amount) } /// Delegate funds to an `Agent`. fn delegate( - who: &Self::AccountId, - agent: &Self::AccountId, + who: Delegator, + agent: Agent, reward_account: &Self::AccountId, amount: Self::Balance, ) -> DispatchResult { Pallet::::register_agent( - RawOrigin::Signed(agent.clone()).into(), + RawOrigin::Signed(agent.clone().get()).into(), reward_account.clone(), )?; // Delegate the funds from who to the `Agent` account. - Pallet::::delegate_to_agent(RawOrigin::Signed(who.clone()).into(), agent.clone(), amount) + Pallet::::delegate_to_agent(RawOrigin::Signed(who.get()).into(), agent.get(), amount) } /// Add more delegation to the `Agent` account. fn delegate_extra( - who: &Self::AccountId, - agent: &Self::AccountId, + who: Delegator, + agent: Agent, amount: Self::Balance, ) -> DispatchResult { - Pallet::::delegate_to_agent(RawOrigin::Signed(who.clone()).into(), agent.clone(), amount) + Pallet::::delegate_to_agent(RawOrigin::Signed(who.get()).into(), agent.get(), amount) } /// Withdraw delegation of `delegator` to `Agent`. @@ -66,33 +66,31 @@ impl DelegationInterface for Pallet { /// If there are funds in `Agent` account that can be withdrawn, then those funds would be /// unlocked/released in the delegator's account. fn withdraw_delegation( - delegator: &Self::AccountId, - agent: &Self::AccountId, + delegator: Delegator, + agent: Agent, amount: Self::Balance, num_slashing_spans: u32, ) -> DispatchResult { Pallet::::release_delegation( - RawOrigin::Signed(agent.clone()).into(), - delegator.clone(), + RawOrigin::Signed(agent.get()).into(), + delegator.get(), amount, num_slashing_spans, ) } - /// Returns true if the `Agent` have any slash pending to be applied. - fn has_pending_slash(agent: &Self::AccountId) -> bool { - Agent::::get(agent) - .map(|d| !d.ledger.pending_slash.is_zero()) - .unwrap_or(false) + /// Returns pending slash of the `agent`. + fn pending_slash(agent: Agent) -> Option { + AgentLedgerOuter::::get(&agent.get()).map(|d| d.ledger.pending_slash).ok() } fn delegator_slash( - agent: &Self::AccountId, - delegator: &Self::AccountId, + agent: Agent, + delegator: Delegator, value: Self::Balance, maybe_reporter: Option, ) -> sp_runtime::DispatchResult { - Pallet::::do_slash(agent.clone(), delegator.clone(), value, maybe_reporter) + Pallet::::do_slash(agent, delegator, value, maybe_reporter) } } @@ -101,32 +99,29 @@ impl DelegationMigrator for Pallet { type AccountId = T::AccountId; fn migrate_nominator_to_agent( - agent: &Self::AccountId, + agent: Agent, reward_account: &Self::AccountId, ) -> DispatchResult { - Pallet::::migrate_to_agent( - RawOrigin::Signed(agent.clone()).into(), - reward_account.clone(), - ) + Pallet::::migrate_to_agent(RawOrigin::Signed(agent.get()).into(), reward_account.clone()) } fn migrate_delegation( - agent: &Self::AccountId, - delegator: &Self::AccountId, + agent: Agent, + delegator: Delegator, value: Self::Balance, ) -> DispatchResult { Pallet::::migrate_delegation( - RawOrigin::Signed(agent.clone()).into(), - delegator.clone(), + RawOrigin::Signed(agent.get()).into(), + delegator.get(), value, ) } /// Only used for testing. #[cfg(feature = "runtime-benchmarks")] - fn drop_agent(agent: &T::AccountId) { - >::remove(agent); + fn drop_agent(agent: Agent) { + >::remove(agent.clone().get()); >::iter() - .filter(|(_, delegation)| delegation.agent == *agent) + .filter(|(_, delegation)| delegation.agent == agent.clone().get()) .for_each(|(delegator, _)| { let _ = T::Currency::release_all( &HoldReason::StakingDelegation.into(), @@ -136,7 +131,7 @@ impl DelegationMigrator for Pallet { >::remove(&delegator); }); - T::CoreStaking::migrate_to_direct_staker(agent); + T::CoreStaking::migrate_to_direct_staker(&agent.get()); } } @@ -158,7 +153,7 @@ impl OnStakingUpdate> for Pallet { fn on_withdraw(stash: &T::AccountId, amount: BalanceOf) { // if there is a withdraw to the agent, then add it to the unclaimed withdrawals. - let _ = Agent::::get(stash) + let _ = AgentLedgerOuter::::get(stash) // can't do anything if there is an overflow error. Just raise a defensive error. .and_then(|agent| agent.add_unclaimed_withdraw(amount).defensive()) .map(|agent| agent.save()); diff --git a/substrate/frame/delegated-staking/src/lib.rs b/substrate/frame/delegated-staking/src/lib.rs index 8581a4a981fe4..4b924bce3a579 100644 --- a/substrate/frame/delegated-staking/src/lib.rs +++ b/substrate/frame/delegated-staking/src/lib.rs @@ -153,7 +153,7 @@ use sp_runtime::{ traits::{AccountIdConversion, CheckedAdd, CheckedSub, Zero}, ArithmeticError, DispatchResult, Perbill, RuntimeDebug, Saturating, }; -use sp_staking::{EraIndex, StakingInterface, StakingUnchecked}; +use sp_staking::{Agent, Delegator, EraIndex, StakingInterface, StakingUnchecked}; use sp_std::{convert::TryInto, prelude::*}; pub type BalanceOf = @@ -345,7 +345,12 @@ pub mod pallet { num_slashing_spans: u32, ) -> DispatchResult { let who = ensure_signed(origin)?; - Self::do_release(&who, &delegator, amount, num_slashing_spans) + Self::do_release( + Agent::from(who), + Delegator::from(delegator), + amount, + num_slashing_spans, + ) } /// Migrate delegated funds that are held in `proxy_delegator` to the claiming `delegator`'s @@ -376,11 +381,11 @@ pub mod pallet { ensure!(Self::is_agent(&agent), Error::::NotAgent); // and has enough delegated balance to migrate. - let proxy_delegator = Self::generate_proxy_delegator(agent); - let balance_remaining = Self::held_balance_of(&proxy_delegator); + let proxy_delegator = Self::generate_proxy_delegator(Agent::from(agent)); + let balance_remaining = Self::held_balance_of(proxy_delegator.clone()); ensure!(balance_remaining >= amount, Error::::NotEnoughFunds); - Self::do_migrate_delegation(&proxy_delegator, &delegator, amount) + Self::do_migrate_delegation(proxy_delegator, Delegator::from(delegator), amount) } /// Delegate given `amount` of tokens to an `Agent` account. @@ -410,10 +415,10 @@ pub mod pallet { ensure!(Self::is_agent(&agent), Error::::NotAgent); // add to delegation. - Self::do_delegate(&delegator, &agent, amount)?; + Self::do_delegate(Delegator::from(delegator), Agent::from(agent.clone()), amount)?; // bond the newly delegated amount to `CoreStaking`. - Self::do_bond(&agent, amount) + Self::do_bond(Agent::from(agent), amount) } } @@ -429,18 +434,18 @@ pub mod pallet { impl Pallet { /// Derive an account from the migrating agent account where the unclaimed delegation funds /// are held. - pub fn generate_proxy_delegator(agent: T::AccountId) -> T::AccountId { - Self::sub_account(AccountType::ProxyDelegator, agent) + pub fn generate_proxy_delegator(agent: Agent) -> Delegator { + Delegator::from(Self::sub_account(AccountType::ProxyDelegator, agent.get())) } /// Derive a (keyless) pot account from the given agent account and account type. - pub(crate) fn sub_account(account_type: AccountType, agent: T::AccountId) -> T::AccountId { - T::PalletId::get().into_sub_account_truncating((account_type, agent.clone())) + fn sub_account(account_type: AccountType, acc: T::AccountId) -> T::AccountId { + T::PalletId::get().into_sub_account_truncating((account_type, acc.clone())) } /// Held balance of a delegator. - pub(crate) fn held_balance_of(who: &T::AccountId) -> BalanceOf { - T::Currency::balance_on_hold(&HoldReason::StakingDelegation.into(), who) + pub(crate) fn held_balance_of(who: Delegator) -> BalanceOf { + T::Currency::balance_on_hold(&HoldReason::StakingDelegation.into(), &who.get()) } /// Returns true if who is registered as an `Agent`. @@ -475,10 +480,10 @@ impl Pallet { // We create a proxy delegator that will keep all the delegation funds until funds are // transferred to actual delegator. - let proxy_delegator = Self::generate_proxy_delegator(who.clone()); + 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); + frame_system::Pallet::::inc_providers(&proxy_delegator.clone().get()); // Get current stake let stake = T::CoreStaking::stake(who)?; @@ -491,11 +496,16 @@ impl Pallet { T::Currency::reducible_balance(who, Preservation::Expendable, Fortitude::Polite); // This should never fail but if it does, it indicates bad state and we abort. - T::Currency::transfer(who, &proxy_delegator, amount_to_transfer, Preservation::Expendable)?; + T::Currency::transfer( + who, + &proxy_delegator.clone().get(), + amount_to_transfer, + Preservation::Expendable, + )?; T::CoreStaking::update_payee(who, reward_account)?; // delegate all transferred funds back to agent. - Self::do_delegate(&proxy_delegator, who, amount_to_transfer)?; + 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. @@ -516,32 +526,36 @@ impl Pallet { } /// Bond `amount` to `agent_acc` in [`Config::CoreStaking`]. - fn do_bond(agent_acc: &T::AccountId, amount: BalanceOf) -> DispatchResult { - let agent = Agent::::get(agent_acc)?; + fn do_bond(agent_acc: Agent, amount: BalanceOf) -> DispatchResult { + let agent_ledger = AgentLedgerOuter::::get(&agent_acc.get())?; - let available_to_bond = agent.available_to_bond(); + let available_to_bond = agent_ledger.available_to_bond(); defensive_assert!(amount == available_to_bond, "not expected value to bond"); - if agent.is_bonded() { - T::CoreStaking::bond_extra(&agent.key, amount) + if agent_ledger.is_bonded() { + T::CoreStaking::bond_extra(&agent_ledger.key, amount) } else { - T::CoreStaking::virtual_bond(&agent.key, amount, agent.reward_account()) + T::CoreStaking::virtual_bond(&agent_ledger.key, amount, agent_ledger.reward_account()) } } /// Delegate `amount` from `delegator` to `agent`. fn do_delegate( - delegator: &T::AccountId, - agent: &T::AccountId, + delegator: Delegator, + agent: Agent, amount: BalanceOf, ) -> DispatchResult { - let mut ledger = AgentLedger::::get(agent).ok_or(Error::::NotAgent)?; + // get inner type + let agent = agent.get(); + let delegator = delegator.get(); + + let mut ledger = AgentLedger::::get(&agent).ok_or(Error::::NotAgent)?; // try to hold the funds. - T::Currency::hold(&HoldReason::StakingDelegation.into(), delegator, amount)?; + 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); + if let Some(existing_delegation) = Delegation::::get(&delegator) { + ensure!(existing_delegation.agent == agent, Error::::InvalidDelegation); existing_delegation .amount .checked_add(&amount) @@ -550,54 +564,54 @@ impl Pallet { amount }; - Delegation::::new(agent, new_delegation_amount).update_or_kill(delegator); + 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); + ledger.update(&agent); - Self::deposit_event(Event::::Delegated { - agent: agent.clone(), - delegator: delegator.clone(), - amount, - }); + Self::deposit_event(Event::::Delegated { agent, delegator, amount }); Ok(()) } /// Release `amount` of delegated funds from `agent` to `delegator`. fn do_release( - who: &T::AccountId, - delegator: &T::AccountId, + who: Agent, + delegator: Delegator, amount: BalanceOf, num_slashing_spans: u32, ) -> DispatchResult { - let mut agent = Agent::::get(who)?; - let mut delegation = Delegation::::get(delegator).ok_or(Error::::NotDelegator)?; + // get inner type + let agent = who.get(); + let delegator = delegator.get(); + + let mut agent_ledger = AgentLedgerOuter::::get(&agent)?; + let mut delegation = Delegation::::get(&delegator).ok_or(Error::::NotDelegator)?; // make sure delegation to be released is sound. - ensure!(&delegation.agent == who, Error::::NotAgent); + ensure!(delegation.agent == agent, Error::::NotAgent); ensure!(delegation.amount >= amount, Error::::NotEnoughFunds); // if we do not already have enough funds to be claimed, try withdraw some more. // keep track if we killed the staker in the process. - let stash_killed = if agent.ledger.unclaimed_withdrawals < amount { + let stash_killed = if agent_ledger.ledger.unclaimed_withdrawals < amount { // withdraw account. - let killed = T::CoreStaking::withdraw_unbonded(who.clone(), num_slashing_spans) + let killed = T::CoreStaking::withdraw_unbonded(agent.clone(), num_slashing_spans) .map_err(|_| Error::::WithdrawFailed)?; // reload agent from storage since withdrawal might have changed the state. - agent = agent.refresh()?; + agent_ledger = agent_ledger.reload()?; Some(killed) } else { None }; // if we still do not have enough funds to release, abort. - ensure!(agent.ledger.unclaimed_withdrawals >= amount, Error::::NotEnoughFunds); + ensure!(agent_ledger.ledger.unclaimed_withdrawals >= amount, Error::::NotEnoughFunds); // Claim withdraw from agent. Kill agent if no delegation left. // TODO: Ideally if there is a register, there should be an unregister that should // clean up the agent. Can be improved in future. - if agent.remove_unclaimed_withdraw(amount)?.update_or_kill()? { + if agent_ledger.remove_unclaimed_withdraw(amount)?.update_or_kill()? { match stash_killed { Some(killed) => { // this implies we did a `CoreStaking::withdraw` before release. Ensure @@ -607,12 +621,12 @@ impl Pallet { None => { // We did not do a `CoreStaking::withdraw` before release. Ensure staker is // already killed in `CoreStaking`. - ensure!(T::CoreStaking::status(who).is_err(), Error::::BadState); + ensure!(T::CoreStaking::status(&agent).is_err(), Error::::BadState); }, } // Remove provider reference for `who`. - let _ = frame_system::Pallet::::dec_providers(who).defensive(); + let _ = frame_system::Pallet::::dec_providers(&agent).defensive(); } // book keep delegation @@ -622,56 +636,56 @@ impl Pallet { .defensive_ok_or(ArithmeticError::Overflow)?; // remove delegator if nothing delegated anymore - delegation.update_or_kill(delegator); + delegation.update_or_kill(&delegator); let released = T::Currency::release( &HoldReason::StakingDelegation.into(), - delegator, + &delegator, amount, Precision::BestEffort, )?; defensive_assert!(released == amount, "hold should have been released fully"); - Self::deposit_event(Event::::Released { - agent: who.clone(), - delegator: delegator.clone(), - amount, - }); + Self::deposit_event(Event::::Released { agent, delegator, amount }); Ok(()) } /// Migrates delegation of `amount` from `source` account to `destination` account. fn do_migrate_delegation( - source_delegator: &T::AccountId, - destination_delegator: &T::AccountId, + source_delegator: Delegator, + destination_delegator: Delegator, amount: BalanceOf, ) -> DispatchResult { + // get inner type + let source_delegator = source_delegator.get(); + let destination_delegator = destination_delegator.get(); + let mut source_delegation = - Delegators::::get(source_delegator).defensive_ok_or(Error::::BadState)?; + Delegators::::get(&source_delegator).defensive_ok_or(Error::::BadState)?; // some checks that must have already been checked before. ensure!(source_delegation.amount >= amount, Error::::NotEnoughFunds); debug_assert!( - !Self::is_delegator(destination_delegator) && !Self::is_agent(destination_delegator) + !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); + Delegation::::new(&agent, amount).update_or_kill(&destination_delegator); source_delegation.amount = source_delegation .amount .checked_sub(&amount) .defensive_ok_or(Error::::BadState)?; - source_delegation.update_or_kill(source_delegator); + source_delegation.update_or_kill(&source_delegator); // release funds from source let released = T::Currency::release( &HoldReason::StakingDelegation.into(), - source_delegator, + &source_delegator, amount, Precision::BestEffort, )?; @@ -680,8 +694,8 @@ impl Pallet { // transfer the released amount to `destination_delegator`. let post_balance = T::Currency::transfer( - source_delegator, - destination_delegator, + &source_delegator, + &destination_delegator, amount, Preservation::Expendable, ) @@ -689,15 +703,15 @@ impl Pallet { // if balance is zero, clear provider for source (proxy) delegator. if post_balance == Zero::zero() { - let _ = frame_system::Pallet::::dec_providers(source_delegator).defensive(); + let _ = frame_system::Pallet::::dec_providers(&source_delegator).defensive(); } // hold the funds again in the new delegator account. - T::Currency::hold(&HoldReason::StakingDelegation.into(), destination_delegator, amount)?; + T::Currency::hold(&HoldReason::StakingDelegation.into(), &destination_delegator, amount)?; Self::deposit_event(Event::::MigratedDelegation { agent, - delegator: destination_delegator.clone(), + delegator: destination_delegator, amount, }); @@ -706,17 +720,21 @@ impl Pallet { /// Take slash `amount` from agent's `pending_slash`counter and apply it to `delegator` account. pub fn do_slash( - agent_acc: T::AccountId, - delegator: T::AccountId, + agent: Agent, + delegator: Delegator, amount: BalanceOf, maybe_reporter: Option, ) -> DispatchResult { - let agent = Agent::::get(&agent_acc)?; + // get inner type + let agent = agent.get(); + let delegator = delegator.get(); + + let agent_ledger = AgentLedgerOuter::::get(&agent)?; // ensure there is something to slash - ensure!(agent.ledger.pending_slash > Zero::zero(), Error::::NothingToSlash); + ensure!(agent_ledger.ledger.pending_slash > Zero::zero(), Error::::NothingToSlash); let mut delegation = >::get(&delegator).ok_or(Error::::NotDelegator)?; - ensure!(delegation.agent == agent_acc, Error::::NotAgent); + ensure!(delegation.agent == agent.clone(), Error::::NotAgent); ensure!(delegation.amount >= amount, Error::::NotEnoughFunds); // slash delegator @@ -728,7 +746,7 @@ impl Pallet { let actual_slash = credit.peek(); // remove the applied slashed amount from agent. - agent.remove_slash(actual_slash).save(); + agent_ledger.remove_slash(actual_slash).save(); delegation.amount = delegation.amount.checked_sub(&actual_slash).ok_or(ArithmeticError::Overflow)?; delegation.update_or_kill(&delegator); @@ -746,15 +764,15 @@ impl Pallet { T::OnSlash::on_unbalanced(credit); - Self::deposit_event(Event::::Slashed { agent: agent_acc, delegator, amount }); + Self::deposit_event(Event::::Slashed { agent, delegator, amount }); Ok(()) } /// Total balance that is available for stake. Includes already staked amount. #[cfg(test)] - pub(crate) fn stakeable_balance(who: &T::AccountId) -> BalanceOf { - Agent::::get(who) + pub(crate) fn stakeable_balance(who: Agent) -> BalanceOf { + AgentLedgerOuter::::get(&who.get()) .map(|agent| agent.ledger.stakeable_balance()) .unwrap_or_default() } diff --git a/substrate/frame/delegated-staking/src/mock.rs b/substrate/frame/delegated-staking/src/mock.rs index b9eaffb970e13..c1875055f2fec 100644 --- a/substrate/frame/delegated-staking/src/mock.rs +++ b/substrate/frame/delegated-staking/src/mock.rs @@ -15,7 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::{self as delegated_staking, types::Agent}; +use crate::{self as delegated_staking, types::AgentLedgerOuter}; use frame_support::{ assert_ok, derive_impl, pallet_prelude::*, @@ -34,7 +34,7 @@ use frame_support::dispatch::RawOrigin; use pallet_staking::{ActiveEra, ActiveEraInfo, CurrentEra}; use sp_core::U256; use sp_runtime::traits::Convert; -use sp_staking::{Stake, StakingInterface}; +use sp_staking::{Agent, Stake, StakingInterface}; pub type T = Runtime; type Block = frame_system::mocking::MockBlock; @@ -309,8 +309,8 @@ pub(crate) fn setup_delegation_stake( } // sanity checks - assert_eq!(DelegatedStaking::stakeable_balance(&agent), delegated_amount); - assert_eq!(Agent::::get(&agent).unwrap().available_to_bond(), 0); + assert_eq!(DelegatedStaking::stakeable_balance(Agent::from(agent)), delegated_amount); + assert_eq!(AgentLedgerOuter::::get(&agent).unwrap().available_to_bond(), 0); delegated_amount } @@ -322,11 +322,11 @@ pub(crate) fn start_era(era: sp_staking::EraIndex) { pub(crate) fn eq_stake(who: AccountId, total: Balance, active: Balance) -> bool { Staking::stake(&who).unwrap() == Stake { total, active } && - get_agent(&who).ledger.stakeable_balance() == total + get_agent_ledger(&who).ledger.stakeable_balance() == total } -pub(crate) fn get_agent(agent: &AccountId) -> Agent { - Agent::::get(agent).expect("delegate should exist") +pub(crate) fn get_agent_ledger(agent: &AccountId) -> AgentLedgerOuter { + AgentLedgerOuter::::get(agent).expect("delegate should exist") } parameter_types! { diff --git a/substrate/frame/delegated-staking/src/tests.rs b/substrate/frame/delegated-staking/src/tests.rs index 6b68726b274cb..d40539d40ddda 100644 --- a/substrate/frame/delegated-staking/src/tests.rs +++ b/substrate/frame/delegated-staking/src/tests.rs @@ -22,7 +22,7 @@ use crate::mock::*; use frame_support::{assert_noop, assert_ok, traits::fungible::InspectHold}; use pallet_nomination_pools::{Error as PoolsError, Event as PoolsEvent}; use pallet_staking::Error as StakingError; -use sp_staking::{DelegationInterface, StakerStatus}; +use sp_staking::{Agent, DelegationInterface, Delegator, StakerStatus}; #[test] fn create_an_agent_with_first_delegator() { @@ -48,12 +48,12 @@ fn create_an_agent_with_first_delegator() { // verify assert!(DelegatedStaking::is_agent(&agent)); - assert_eq!(DelegatedStaking::stakeable_balance(&agent), 100); + assert_eq!(DelegatedStaking::stakeable_balance(Agent::from(agent)), 100); assert_eq!( Balances::balance_on_hold(&HoldReason::StakingDelegation.into(), &delegator), 100 ); - assert_eq!(DelegatedStaking::held_balance_of(&delegator), 100); + assert_eq!(DelegatedStaking::held_balance_of(Delegator::from(delegator)), 100); }); } @@ -102,7 +102,7 @@ fn create_multiple_delegators() { // stakeable balance is 0 for non agent fund(&agent, 1000); assert!(!DelegatedStaking::is_agent(&agent)); - assert_eq!(DelegatedStaking::stakeable_balance(&agent), 0); + assert_eq!(DelegatedStaking::stakeable_balance(Agent::from(agent)), 0); // set intention to accept delegation. assert_ok!(DelegatedStaking::register_agent( @@ -124,7 +124,7 @@ fn create_multiple_delegators() { // verify assert!(DelegatedStaking::is_agent(&agent)); - assert_eq!(DelegatedStaking::stakeable_balance(&agent), 100 * 100); + assert_eq!(DelegatedStaking::stakeable_balance(Agent::from(agent)), 100 * 100); }); } @@ -260,39 +260,55 @@ fn apply_pending_slash() { // agent cannot slash an account that is not its delegator. setup_delegation_stake(210, 211, (351..=352).collect(), 100, 0); assert_noop!( - ::delegator_slash(&agent, &351, 1, Some(400)), + ::delegator_slash( + Agent::from(agent), + Delegator::from(351), + 1, + Some(400) + ), Error::::NotAgent ); // or a non delegator account fund(&353, 100); assert_noop!( - ::delegator_slash(&agent, &353, 1, Some(400)), + ::delegator_slash( + Agent::from(agent), + Delegator::from(353), + 1, + Some(400) + ), Error::::NotDelegator ); // ensure bookkept pending slash is correct. - assert_eq!(get_agent(&agent).ledger.pending_slash, total_staked / 2); + assert_eq!(get_agent_ledger(&agent).ledger.pending_slash, total_staked / 2); let mut old_reporter_balance = Balances::free_balance(reporter); // lets apply the pending slash on delegators. for i in delegators { // balance before slash - let initial_pending_slash = get_agent(&agent).ledger.pending_slash; + let initial_pending_slash = get_agent_ledger(&agent).ledger.pending_slash; assert!(initial_pending_slash > 0); - let unslashed_balance = DelegatedStaking::held_balance_of(&i); + let unslashed_balance = DelegatedStaking::held_balance_of(Delegator::from(i)); let slash = unslashed_balance / 2; // slash half of delegator's delegation. assert_ok!(::delegator_slash( - &agent, - &i, + Agent::from(agent), + Delegator::from(i), slash, Some(400) )); // balance after slash. - assert_eq!(DelegatedStaking::held_balance_of(&i), unslashed_balance - slash); + assert_eq!( + DelegatedStaking::held_balance_of(Delegator::from(i)), + unslashed_balance - slash + ); // pending slash is reduced by the amount slashed. - assert_eq!(get_agent(&agent).ledger.pending_slash, initial_pending_slash - slash); + assert_eq!( + get_agent_ledger(&agent).ledger.pending_slash, + initial_pending_slash - slash + ); // reporter get 10% of the slash amount. assert_eq!( Balances::free_balance(reporter) - old_reporter_balance, @@ -303,11 +319,16 @@ fn apply_pending_slash() { } // nothing to slash anymore - assert_eq!(get_agent(&agent).ledger.pending_slash, 0); + assert_eq!(get_agent_ledger(&agent).ledger.pending_slash, 0); // cannot slash anymore assert_noop!( - ::delegator_slash(&agent, &350, 1, None), + ::delegator_slash( + Agent::from(agent), + Delegator::from(350), + 1, + None + ), Error::::NothingToSlash ); }); @@ -332,7 +353,7 @@ mod staking_integration { RawOrigin::Signed(agent).into(), reward_acc )); - assert_eq!(DelegatedStaking::stakeable_balance(&agent), 0); + assert_eq!(DelegatedStaking::stakeable_balance(Agent::from(agent)), 0); let mut delegated_balance: Balance = 0; @@ -349,9 +370,12 @@ mod staking_integration { Balances::balance_on_hold(&HoldReason::StakingDelegation.into(), &delegator), 100 ); - assert_eq!(DelegatedStaking::delegator_balance(&delegator), 100); + assert_eq!( + DelegatedStaking::delegator_balance(Delegator::from(delegator)).unwrap(), + 100 + ); - let agent_obj = get_agent(&agent); + let agent_obj = get_agent_ledger(&agent); assert_eq!(agent_obj.ledger.stakeable_balance(), delegated_balance); assert_eq!(agent_obj.available_to_bond(), 0); assert_eq!(agent_obj.bonded_stake(), delegated_balance); @@ -403,9 +427,9 @@ mod staking_integration { Error::::NotEnoughFunds ); - assert_eq!(get_agent(&agent).available_to_bond(), 0); + assert_eq!(get_agent_ledger(&agent).available_to_bond(), 0); // full amount is still delegated - assert_eq!(get_agent(&agent).ledger.effective_balance(), total_staked); + assert_eq!(get_agent_ledger(&agent).ledger.effective_balance(), total_staked); start_era(5); // at era 5, 50 tokens are withdrawable, cannot withdraw more. @@ -491,7 +515,7 @@ mod staking_integration { start_era(i); assert_ok!(Staking::unbond(RawOrigin::Signed(agent).into(), 10)); // no withdrawals from core staking yet. - assert_eq!(get_agent(&agent).ledger.unclaimed_withdrawals, 0); + assert_eq!(get_agent_ledger(&agent).ledger.unclaimed_withdrawals, 0); } // another unbond would trigger withdrawal @@ -500,7 +524,7 @@ mod staking_integration { // 8 previous unbonds would be withdrawn as they were already unlocked. Unlocking period // is 3 eras. - assert_eq!(get_agent(&agent).ledger.unclaimed_withdrawals, 8 * 10); + assert_eq!(get_agent_ledger(&agent).ledger.unclaimed_withdrawals, 8 * 10); // release some delegation now. assert_ok!(DelegatedStaking::release_delegation( @@ -509,7 +533,7 @@ mod staking_integration { 40, 0 )); - assert_eq!(get_agent(&agent).ledger.unclaimed_withdrawals, 80 - 40); + assert_eq!(get_agent_ledger(&agent).ledger.unclaimed_withdrawals, 80 - 40); // cannot release more than available assert_noop!( @@ -523,7 +547,7 @@ mod staking_integration { 0 )); - assert_eq!(DelegatedStaking::held_balance_of(&300), 100 - 80); + assert_eq!(DelegatedStaking::held_balance_of(Delegator::from(300)), 100 - 80); }); } @@ -561,8 +585,8 @@ mod staking_integration { // amount is staked correctly assert!(eq_stake(200, 100, 100)); - assert_eq!(get_agent(&200).available_to_bond(), 0); - assert_eq!(get_agent(&200).ledger.effective_balance(), 100); + assert_eq!(get_agent_ledger(&200).available_to_bond(), 0); + assert_eq!(get_agent_ledger(&200).ledger.effective_balance(), 100); // free balance of delegate is untouched assert_eq!(Balances::free_balance(200), balance_200); @@ -624,7 +648,8 @@ mod staking_integration { // 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(200); + let proxy_delegator = + DelegatedStaking::generate_proxy_delegator(Agent::from(200)).get(); assert_ok!(DelegatedStaking::migrate_to_agent(RawOrigin::Signed(200).into(), 201)); @@ -637,9 +662,12 @@ mod staking_integration { // 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(&200).ledger.effective_balance(), agent_amount); - assert_eq!(get_agent(&200).available_to_bond(), 0); - assert_eq!(get_agent(&200).ledger.unclaimed_withdrawals, agent_amount - staked_amount); + assert_eq!(get_agent_ledger(&200).ledger.effective_balance(), agent_amount); + assert_eq!(get_agent_ledger(&200).available_to_bond(), 0); + assert_eq!( + get_agent_ledger(&200).ledger.unclaimed_withdrawals, + agent_amount - staked_amount + ); // now lets migrate the delegators let delegator_share = agent_amount / 4; @@ -668,10 +696,10 @@ mod staking_integration { // delegate stake is unchanged. assert_eq!(Staking::stake(&200).unwrap(), init_stake); - assert_eq!(get_agent(&200).ledger.effective_balance(), agent_amount); - assert_eq!(get_agent(&200).available_to_bond(), 0); + assert_eq!(get_agent_ledger(&200).ledger.effective_balance(), agent_amount); + assert_eq!(get_agent_ledger(&200).available_to_bond(), 0); assert_eq!( - get_agent(&200).ledger.unclaimed_withdrawals, + get_agent_ledger(&200).ledger.unclaimed_withdrawals, agent_amount - staked_amount ); } @@ -697,7 +725,7 @@ mod pool_integration { let delegate_amount = 200; // nothing held initially - assert_eq!(DelegatedStaking::held_balance_of(&creator), 0); + assert_eq!(DelegatedStaking::held_balance_of(Delegator::from(creator)), 0); // create pool assert_ok!(Pools::create( @@ -709,10 +737,13 @@ mod pool_integration { )); // correct amount is locked in depositor's account. - assert_eq!(DelegatedStaking::held_balance_of(&creator), delegate_amount); + assert_eq!( + DelegatedStaking::held_balance_of(Delegator::from(creator)), + delegate_amount + ); let pool_account = Pools::generate_bonded_account(1); - let agent = get_agent(&pool_account); + let agent = get_agent_ledger(&pool_account); // verify state assert_eq!(agent.ledger.effective_balance(), delegate_amount); @@ -733,19 +764,19 @@ mod pool_integration { let delegator: AccountId = 300; fund(&delegator, 500); // nothing held initially - assert_eq!(DelegatedStaking::held_balance_of(&delegator), 0); + assert_eq!(DelegatedStaking::held_balance_of(Delegator::from(delegator)), 0); // delegator joins pool assert_ok!(Pools::join(RawOrigin::Signed(delegator).into(), 100, pool_id)); staked_amount += 100; // correct amount is locked in depositor's account. - assert_eq!(DelegatedStaking::held_balance_of(&delegator), 100); + assert_eq!(DelegatedStaking::held_balance_of(Delegator::from(delegator)), 100); // delegator is not actively exposed to core staking. assert_eq!(Staking::status(&delegator), Err(StakingError::::NotStash.into())); - let pool_agent = get_agent(&Pools::generate_bonded_account(1)); + let pool_agent = get_agent_ledger(&Pools::generate_bonded_account(1)); // verify state assert_eq!(pool_agent.ledger.effective_balance(), staked_amount); assert_eq!(pool_agent.bonded_stake(), staked_amount); @@ -763,10 +794,10 @@ mod pool_integration { fund(&i, 500); assert_ok!(Pools::join(RawOrigin::Signed(i).into(), 100 + i, pool_id)); staked_amount += 100 + i; - assert_eq!(DelegatedStaking::held_balance_of(&i), 100 + i); + assert_eq!(DelegatedStaking::held_balance_of(Delegator::from(i)), 100 + i); } - let pool_agent = pool_agent.refresh().unwrap(); + let pool_agent = pool_agent.reload().unwrap(); assert_eq!(pool_agent.ledger.effective_balance(), staked_amount); assert_eq!(pool_agent.bonded_stake(), staked_amount); assert_eq!(pool_agent.available_to_bond(), 0); @@ -812,7 +843,8 @@ mod pool_integration { // claim rewards for i in 300..320 { let pre_balance = Balances::free_balance(i); - let delegator_staked_balance = DelegatedStaking::held_balance_of(&i); + let delegator_staked_balance = + DelegatedStaking::held_balance_of(Delegator::from(i)); // payout reward assert_ok!(Pools::claim_payout(RawOrigin::Signed(i).into())); @@ -880,7 +912,7 @@ mod pool_integration { // at era 5, 301 can withdraw. System::reset_events(); - let held_301 = DelegatedStaking::held_balance_of(&301); + let held_301 = DelegatedStaking::held_balance_of(Delegator::from(301)); let free_301 = Balances::free_balance(301); assert_ok!(Pools::withdraw_unbonded(RawOrigin::Signed(301).into(), 301, 0)); @@ -892,7 +924,7 @@ mod pool_integration { pool_events_since_last_call(), vec![PoolsEvent::Withdrawn { member: 301, pool_id, balance: 50, points: 50 }] ); - assert_eq!(DelegatedStaking::held_balance_of(&301), held_301 - 50); + assert_eq!(DelegatedStaking::held_balance_of(Delegator::from(301)), held_301 - 50); assert_eq!(Balances::free_balance(301), free_301 + 50); start_era(7); @@ -1069,6 +1101,11 @@ mod pool_integration { BondedPools::::get(1).unwrap().points, creator_stake + delegator_stake * 6 - delegator_stake * 3 ); + + // pool has currently no pending slash + assert_eq!(Pools::api_pool_pending_slash(pool_id), 0); + + // slash the pool partially pallet_staking::slashing::do_slash::( &pool_acc, 500, @@ -1077,6 +1114,9 @@ mod pool_integration { 3, ); + // pool has now pending slash of 500. + assert_eq!(Pools::api_pool_pending_slash(pool_id), 500); + assert_eq!( pool_events_since_last_call(), vec![ @@ -1091,9 +1131,9 @@ mod pool_integration { ); // slash is lazy and balance is still locked in user's accounts. - assert_eq!(DelegatedStaking::held_balance_of(&creator), creator_stake); + assert_eq!(DelegatedStaking::held_balance_of(Delegator::from(creator)), creator_stake); for i in 300..306 { - assert_eq!(DelegatedStaking::held_balance_of(&i), delegator_stake); + assert_eq!(DelegatedStaking::held_balance_of(Delegator::from(i)), delegator_stake); } assert_eq!( get_pool_agent(pool_id).ledger.effective_balance(), @@ -1128,7 +1168,7 @@ mod pool_integration { ] ); assert_eq!(get_pool_agent(pool_id).ledger.pending_slash, pre_pending_slash - 50); - assert_eq!(DelegatedStaking::held_balance_of(&i), 0); + assert_eq!(DelegatedStaking::held_balance_of(Delegator::from(i)), 0); assert_eq!(Balances::free_balance(i) - pre_balance, 50); } @@ -1139,19 +1179,38 @@ mod pool_integration { for i in 303..306 { let pre_pending_slash = get_pool_agent(pool_id).ledger.pending_slash; + // pool api returns correct pending slash. + assert_eq!(Pools::api_pool_pending_slash(pool_id), pre_pending_slash); + // delegator has pending slash of 50. + assert_eq!(Pools::api_member_pending_slash(i), 50); + // apply slash assert_ok!(Pools::apply_slash(RawOrigin::Signed(slash_reporter).into(), i)); + // nothing pending anymore. + assert_eq!(Pools::api_member_pending_slash(i), 0); // each member is slashed 50% of 100 = 50. assert_eq!(get_pool_agent(pool_id).ledger.pending_slash, pre_pending_slash - 50); + // pool api returns correctly as well. + assert_eq!(Pools::api_pool_pending_slash(pool_id), pre_pending_slash - 50); // left with 50. - assert_eq!(DelegatedStaking::held_balance_of(&i), 50); + assert_eq!(DelegatedStaking::held_balance_of(Delegator::from(i)), 50); } + + // pool has still pending slash of creator. + assert_eq!(Pools::api_pool_pending_slash(pool_id), 250); + // reporter is paid SlashRewardFraction of the slash, i.e. 10% of 50 = 5 assert_eq!(Balances::free_balance(slash_reporter), 100 + 5 * 3); + // creator has pending slash. + assert_eq!(Pools::api_member_pending_slash(creator), 250); // slash creator assert_ok!(Pools::apply_slash(RawOrigin::Signed(slash_reporter).into(), creator)); + // no pending slash anymore. + assert_eq!(Pools::api_member_pending_slash(creator), 0); + // all slash should be applied now. assert_eq!(get_pool_agent(pool_id).ledger.pending_slash, 0); + assert_eq!(Pools::api_pool_pending_slash(pool_id), 0); // for creator, 50% of stake should be slashed (250), 10% of which should go to reporter // (25). assert_eq!(Balances::free_balance(slash_reporter), 115 + 25); @@ -1178,7 +1237,7 @@ mod pool_integration { } } - fn get_pool_agent(pool_id: u32) -> Agent { - get_agent(&Pools::generate_bonded_account(pool_id)) + fn get_pool_agent(pool_id: u32) -> AgentLedgerOuter { + get_agent_ledger(&Pools::generate_bonded_account(pool_id)) } } diff --git a/substrate/frame/delegated-staking/src/types.rs b/substrate/frame/delegated-staking/src/types.rs index 958d81c294aa9..24b4573565441 100644 --- a/substrate/frame/delegated-staking/src/types.rs +++ b/substrate/frame/delegated-staking/src/types.rs @@ -143,18 +143,18 @@ impl AgentLedger { /// Wrapper around `AgentLedger` to provide some helper functions to mutate the ledger. #[derive(Clone)] -pub struct Agent { +pub struct AgentLedgerOuter { /// storage key pub key: T::AccountId, /// storage value pub ledger: AgentLedger, } -impl Agent { +impl AgentLedgerOuter { /// Get `Agent` from storage if it exists or return an error. - pub(crate) fn get(agent: &T::AccountId) -> Result, DispatchError> { + pub(crate) fn get(agent: &T::AccountId) -> Result, DispatchError> { let ledger = AgentLedger::::get(agent).ok_or(Error::::NotAgent)?; - Ok(Agent { key: agent.clone(), ledger }) + Ok(AgentLedgerOuter { key: agent.clone(), ledger }) } /// Remove funds that are withdrawn from [Config::CoreStaking] but not claimed by a delegator. @@ -176,7 +176,7 @@ impl Agent { .checked_sub(&amount) .defensive_ok_or(ArithmeticError::Overflow)?; - Ok(Agent { + Ok(AgentLedgerOuter { ledger: AgentLedger { total_delegated: new_total_delegated, unclaimed_withdrawals: new_unclaimed_withdrawals, @@ -197,7 +197,7 @@ impl Agent { .checked_add(&amount) .defensive_ok_or(ArithmeticError::Overflow)?; - Ok(Agent { + Ok(AgentLedgerOuter { ledger: AgentLedger { unclaimed_withdrawals: new_unclaimed_withdrawals, ..self.ledger }, ..self }) @@ -224,7 +224,10 @@ impl Agent { let pending_slash = self.ledger.pending_slash.defensive_saturating_sub(amount); let total_delegated = self.ledger.total_delegated.defensive_saturating_sub(amount); - Agent { ledger: AgentLedger { pending_slash, total_delegated, ..self.ledger }, ..self } + AgentLedgerOuter { + ledger: AgentLedger { pending_slash, total_delegated, ..self.ledger }, + ..self + } } /// Get the total stake of agent bonded in [`Config::CoreStaking`]. @@ -270,7 +273,7 @@ impl Agent { } /// Reloads self from storage. - pub(crate) fn refresh(self) -> Result, DispatchError> { + pub(crate) fn reload(self) -> Result, DispatchError> { Self::get(&self.key) } diff --git a/substrate/frame/nomination-pools/benchmarking/src/inner.rs b/substrate/frame/nomination-pools/benchmarking/src/inner.rs index 43de0fddb8b5f..b8c978945e9ee 100644 --- a/substrate/frame/nomination-pools/benchmarking/src/inner.rs +++ b/substrate/frame/nomination-pools/benchmarking/src/inner.rs @@ -29,7 +29,7 @@ use frame_support::{ }; use frame_system::RawOrigin as RuntimeOrigin; use pallet_nomination_pools::{ - adapter::{StakeStrategy, StakeStrategyType}, + adapter::{Member, Pool, StakeStrategy, StakeStrategyType}, BalanceOf, BondExtra, BondedPoolInner, BondedPools, ClaimPermission, ClaimPermissions, Commission, CommissionChangeRate, CommissionClaimPermission, ConfigOp, GlobalMaxCommission, MaxPoolMembers, MaxPoolMembersPerPool, MaxPools, Metadata, MinCreateBond, MinJoinBond, @@ -116,7 +116,7 @@ fn migrate_to_transfer_stake(pool_id: PoolId) { } let pool_acc = Pools::::generate_bonded_account(pool_id); // drop the agent and its associated delegators . - T::StakeAdapter::remove_as_agent(&pool_acc); + T::StakeAdapter::remove_as_agent(Pool::from(pool_acc.clone())); // tranfer funds from all members to the pool account. PoolMembers::::iter() @@ -139,8 +139,12 @@ fn vote_to_balance( vote.try_into().map_err(|_| "could not convert u64 to Balance") } -fn is_transfer_stake_strategy() -> bool { - T::StakeAdapter::strategy_type() == StakeStrategyType::Transfer +/// `assertion` should strictly be true if the adapter is using `Delegate` strategy and strictly +/// false if the adapter is not using `Delegate` strategy. +fn assert_if_delegate(assertion: bool) { + let legacy_adapter_used = T::StakeAdapter::strategy_type() != StakeStrategyType::Delegate; + // one and only one of the two should be true. + assert!(assertion ^ legacy_adapter_used); } #[allow(unused)] @@ -182,7 +186,7 @@ impl ListScenario { create_pool_account::(USER_SEED + 1, origin_weight, Some(Perbill::from_percent(50))); T::StakeAdapter::nominate( - &pool_origin1, + Pool::from(pool_origin1.clone()), // NOTE: these don't really need to be validators. vec![account("random_validator", 0, USER_SEED)], )?; @@ -191,7 +195,7 @@ impl ListScenario { create_pool_account::(USER_SEED + 2, origin_weight, Some(Perbill::from_percent(50))); T::StakeAdapter::nominate( - &pool_origin2, + Pool::from(pool_origin2.clone()), vec![account("random_validator", 0, USER_SEED)].clone(), )?; @@ -208,7 +212,10 @@ impl ListScenario { let (_, pool_dest1) = create_pool_account::(USER_SEED + 3, dest_weight, Some(Perbill::from_percent(50))); - T::StakeAdapter::nominate(&pool_dest1, vec![account("random_validator", 0, USER_SEED)])?; + T::StakeAdapter::nominate( + Pool::from(pool_dest1.clone()), + vec![account("random_validator", 0, USER_SEED)], + )?; let weight_of = pallet_staking::Pallet::::weight_of_fn(); assert_eq!(vote_to_balance::(weight_of(&pool_origin1)).unwrap(), origin_weight); @@ -234,11 +241,11 @@ impl ListScenario { self.origin1_member = Some(joiner.clone()); CurrencyOf::::set_balance(&joiner, amount * 2u32.into()); - let original_bonded = T::StakeAdapter::active_stake(&self.origin1); + let original_bonded = T::StakeAdapter::active_stake(Pool::from(self.origin1.clone())); // Unbond `amount` from the underlying pool account so when the member joins // we will maintain `current_bonded`. - T::StakeAdapter::unbond(&self.origin1, amount) + T::StakeAdapter::unbond(Pool::from(self.origin1.clone()), amount) .expect("the pool was created in `Self::new`."); // Account pool points for the unbonded balance. @@ -275,7 +282,7 @@ frame_benchmarking::benchmarks! { // setup the worst case list scenario. let scenario = ListScenario::::new(origin_weight, true)?; assert_eq!( - T::StakeAdapter::active_stake(&scenario.origin1), + T::StakeAdapter::active_stake(Pool::from(scenario.origin1.clone())), origin_weight ); @@ -290,7 +297,7 @@ frame_benchmarking::benchmarks! { verify { assert_eq!(CurrencyOf::::balance(&joiner), joiner_free - max_additional); assert_eq!( - T::StakeAdapter::active_stake(&scenario.origin1), + T::StakeAdapter::active_stake(Pool::from(scenario.origin1)), scenario.dest_weight ); } @@ -305,7 +312,7 @@ frame_benchmarking::benchmarks! { }: bond_extra(RuntimeOrigin::Signed(scenario.creator1.clone()), BondExtra::FreeBalance(extra)) verify { assert!( - T::StakeAdapter::active_stake(&scenario.origin1) >= + T::StakeAdapter::active_stake(Pool::from(scenario.origin1)) >= scenario.dest_weight ); } @@ -329,7 +336,7 @@ frame_benchmarking::benchmarks! { verify { // commission of 50% deducted here. assert!( - T::StakeAdapter::active_stake(&scenario.origin1) >= + T::StakeAdapter::active_stake(Pool::from(scenario.origin1)) >= scenario.dest_weight / 2u32.into() ); } @@ -383,7 +390,7 @@ frame_benchmarking::benchmarks! { whitelist_account!(member_id); }: _(RuntimeOrigin::Signed(member_id.clone()), member_id_lookup, all_points) verify { - let bonded_after = T::StakeAdapter::active_stake(&scenario.origin1); + let bonded_after = T::StakeAdapter::active_stake(Pool::from(scenario.origin1)); // We at least went down to the destination bag assert!(bonded_after <= scenario.dest_weight); let member = PoolMembers::::get( @@ -414,7 +421,7 @@ frame_benchmarking::benchmarks! { // Sanity check join worked assert_eq!( - T::StakeAdapter::active_stake(&pool_account), + T::StakeAdapter::active_stake(Pool::from(pool_account.clone())), min_create_bond + min_join_bond ); assert_eq!(CurrencyOf::::balance(&joiner), min_join_bond); @@ -424,7 +431,7 @@ frame_benchmarking::benchmarks! { // Sanity check that unbond worked assert_eq!( - T::StakeAdapter::active_stake(&pool_account), + T::StakeAdapter::active_stake(Pool::from(pool_account.clone())), min_create_bond ); assert_eq!(pallet_staking::Ledger::::get(&pool_account).unwrap().unlocking.len(), 1); @@ -457,7 +464,7 @@ frame_benchmarking::benchmarks! { // Sanity check join worked assert_eq!( - T::StakeAdapter::active_stake(&pool_account), + T::StakeAdapter::active_stake(Pool::from(pool_account.clone())), min_create_bond + min_join_bond ); assert_eq!(CurrencyOf::::balance(&joiner), min_join_bond); @@ -468,7 +475,7 @@ frame_benchmarking::benchmarks! { // Sanity check that unbond worked assert_eq!( - T::StakeAdapter::active_stake(&pool_account), + T::StakeAdapter::active_stake(Pool::from(pool_account.clone())), min_create_bond ); assert_eq!(pallet_staking::Ledger::::get(&pool_account).unwrap().unlocking.len(), 1); @@ -514,12 +521,12 @@ frame_benchmarking::benchmarks! { // Sanity check that unbond worked assert_eq!( - T::StakeAdapter::active_stake(&pool_account), + T::StakeAdapter::active_stake(Pool::from(pool_account.clone())), Zero::zero() ); assert_eq!( - T::StakeAdapter::total_balance(&pool_account), - min_create_bond + T::StakeAdapter::total_balance(Pool::from(pool_account.clone())), + Some(min_create_bond) ); assert_eq!(pallet_staking::Ledger::::get(&pool_account).unwrap().unlocking.len(), 1); @@ -594,7 +601,7 @@ frame_benchmarking::benchmarks! { } ); assert_eq!( - T::StakeAdapter::active_stake(&Pools::::generate_bonded_account(1)), + T::StakeAdapter::active_stake(Pool::from(Pools::::generate_bonded_account(1))), min_create_bond ); } @@ -634,7 +641,7 @@ frame_benchmarking::benchmarks! { } ); assert_eq!( - T::StakeAdapter::active_stake(&Pools::::generate_bonded_account(1)), + T::StakeAdapter::active_stake(Pool::from(Pools::::generate_bonded_account(1))), min_create_bond ); } @@ -719,13 +726,13 @@ frame_benchmarking::benchmarks! { .map(|i| account("stash", USER_SEED, i)) .collect(); - assert_ok!(T::StakeAdapter::nominate(&pool_account, validators)); - assert!(T::StakeAdapter::nominations(&Pools::::generate_bonded_account(1)).is_some()); + assert_ok!(T::StakeAdapter::nominate(Pool::from(pool_account.clone()), validators)); + assert!(T::StakeAdapter::nominations(Pool::from(pool_account.clone())).is_some()); whitelist_account!(depositor); }:_(RuntimeOrigin::Signed(depositor.clone()), 1) verify { - assert!(T::StakeAdapter::nominations(&Pools::::generate_bonded_account(1)).is_none()); + assert!(T::StakeAdapter::nominations(Pool::from(pool_account.clone())).is_none()); } set_commission { @@ -824,7 +831,7 @@ frame_benchmarking::benchmarks! { // Sanity check join worked assert_eq!( - T::StakeAdapter::active_stake(&pool_account), + T::StakeAdapter::active_stake(Pool::from(pool_account.clone())), min_create_bond + min_join_bond ); }:_(RuntimeOrigin::Signed(joiner.clone()), ClaimPermission::Permissioned) @@ -888,7 +895,7 @@ frame_benchmarking::benchmarks! { // verify user balance in the pool. assert_eq!(PoolMembers::::get(&depositor).unwrap().total_balance(), deposit_amount); // verify delegated balance. - assert!(is_transfer_stake_strategy::() || T::StakeAdapter::member_delegation_balance(&depositor) == deposit_amount); + assert_if_delegate::(T::StakeAdapter::member_delegation_balance(Member::from(depositor.clone())) == Some(deposit_amount)); // ugly type conversion between balances of pallet staking and pools (which really are same // type). Maybe there is a better way? @@ -906,7 +913,7 @@ frame_benchmarking::benchmarks! { // verify user balance is slashed in the pool. assert_eq!(PoolMembers::::get(&depositor).unwrap().total_balance(), deposit_amount/2u32.into()); // verify delegated balance are not yet slashed. - assert!(is_transfer_stake_strategy::() || T::StakeAdapter::member_delegation_balance(&depositor) == deposit_amount); + assert_if_delegate::(T::StakeAdapter::member_delegation_balance(Member::from(depositor.clone())) == Some(deposit_amount)); // Fill member's sub pools for the worst case. for i in 1..(T::MaxUnbonding::get() + 1) { @@ -920,14 +927,12 @@ frame_benchmarking::benchmarks! { whitelist_account!(depositor); }: { - let res = Pools::::apply_slash(RuntimeOrigin::Signed(slash_reporter.clone()).into(), depositor_lookup.clone()); - // for transfer stake strategy, apply slash would error, otherwise success. - assert!(is_transfer_stake_strategy::() ^ res.is_ok()); + assert_if_delegate::(Pools::::apply_slash(RuntimeOrigin::Signed(slash_reporter.clone()).into(), depositor_lookup.clone()).is_ok()); } verify { // verify balances are correct and slash applied. assert_eq!(PoolMembers::::get(&depositor).unwrap().total_balance(), deposit_amount/2u32.into()); - assert!(is_transfer_stake_strategy::() || T::StakeAdapter::member_delegation_balance(&depositor) == deposit_amount/2u32.into()); + assert_if_delegate::(T::StakeAdapter::member_delegation_balance(Member::from(depositor.clone())) == Some(deposit_amount/2u32.into())); } apply_slash_fail { @@ -979,13 +984,11 @@ frame_benchmarking::benchmarks! { // migrate pool to transfer stake. let _ = migrate_to_transfer_stake::(1); }: { - // Try migrate to `DelegateStake`. Would succeed only if `DelegateStake` strategy is used. - let res = Pools::::migrate_pool_to_delegate_stake(RuntimeOrigin::Signed(depositor.clone()).into(), 1u32.into()); - assert!(is_transfer_stake_strategy::() ^ res.is_ok()); + assert_if_delegate::(Pools::::migrate_pool_to_delegate_stake(RuntimeOrigin::Signed(depositor.clone()).into(), 1u32.into()).is_ok()); } verify { // this queries agent balance if `DelegateStake` strategy. - assert!(T::StakeAdapter::total_balance(&pool_account) == deposit_amount); + assert!(T::StakeAdapter::total_balance(Pool::from(pool_account.clone())) == Some(deposit_amount)); } migrate_delegation { @@ -998,22 +1001,20 @@ frame_benchmarking::benchmarks! { let _ = migrate_to_transfer_stake::(1); // Now migrate pool to delegate stake keeping delegators unmigrated. - let migration_res = Pools::::migrate_pool_to_delegate_stake(RuntimeOrigin::Signed(depositor.clone()).into(), 1u32.into()); - assert!(is_transfer_stake_strategy::() ^ migration_res.is_ok()); + assert_if_delegate::(Pools::::migrate_pool_to_delegate_stake(RuntimeOrigin::Signed(depositor.clone()).into(), 1u32.into()).is_ok()); - // verify balances that we will check again later. - assert!(T::StakeAdapter::member_delegation_balance(&depositor) == Zero::zero()); + // delegation does not exist. + assert!(T::StakeAdapter::member_delegation_balance(Member::from(depositor.clone())).is_none()); + // contribution exists in the pool. assert_eq!(PoolMembers::::get(&depositor).unwrap().total_balance(), deposit_amount); whitelist_account!(depositor); }: { - let res = Pools::::migrate_delegation(RuntimeOrigin::Signed(depositor.clone()).into(), depositor_lookup.clone()); - // for transfer stake strategy, apply slash would error, otherwise success. - assert!(is_transfer_stake_strategy::() ^ res.is_ok()); + assert_if_delegate::(Pools::::migrate_delegation(RuntimeOrigin::Signed(depositor.clone()).into(), depositor_lookup.clone()).is_ok()); } verify { // verify balances once more. - assert!(is_transfer_stake_strategy::() || T::StakeAdapter::member_delegation_balance(&depositor) == deposit_amount); + assert_if_delegate::(T::StakeAdapter::member_delegation_balance(Member::from(depositor.clone())) == Some(deposit_amount)); assert_eq!(PoolMembers::::get(&depositor).unwrap().total_balance(), deposit_amount); } diff --git a/substrate/frame/nomination-pools/runtime-api/src/lib.rs b/substrate/frame/nomination-pools/runtime-api/src/lib.rs index 881c3c36331b6..67627e0acb13d 100644 --- a/substrate/frame/nomination-pools/runtime-api/src/lib.rs +++ b/substrate/frame/nomination-pools/runtime-api/src/lib.rs @@ -38,5 +38,30 @@ sp_api::decl_runtime_apis! { /// Returns the equivalent points of `new_funds` for a given pool. fn balance_to_points(pool_id: PoolId, new_funds: Balance) -> Balance; + + /// Returns the pending slash for a given pool. + fn pool_pending_slash(pool_id: PoolId) -> Balance; + + /// Returns the pending slash for a given pool member. + fn member_pending_slash(member: AccountId) -> Balance; + + /// Returns true if the pool with `pool_id` needs migration. + /// + /// This can happen when the `pallet-nomination-pools` has switched to using strategy + /// [`DelegateStake`](pallet_nomination_pools::adapter::DelegateStake) but the pool + /// still has funds that were staked using the older strategy + /// [TransferStake](pallet_nomination_pools::adapter::TransferStake). Use + /// [`migrate_pool_to_delegate_stake`](pallet_nomination_pools::Call::migrate_pool_to_delegate_stake) + /// to migrate the pool. + fn pool_needs_delegate_migration(pool_id: PoolId) -> bool; + + /// Returns true if the delegated funds of the pool `member` needs migration. + /// + /// Once a pool has successfully migrated to the strategy + /// [`DelegateStake`](pallet_nomination_pools::adapter::DelegateStake), the funds of the + /// member can be migrated from pool account to the member's account. Use + /// [`migrate_delegation`](pallet_nomination_pools::Call::migrate_delegation) + /// to migrate the funds of the pool member. + fn member_needs_delegate_migration(member: AccountId) -> bool; } } diff --git a/substrate/frame/nomination-pools/src/adapter.rs b/substrate/frame/nomination-pools/src/adapter.rs index caf4671191d88..4809fbc0e9da0 100644 --- a/substrate/frame/nomination-pools/src/adapter.rs +++ b/substrate/frame/nomination-pools/src/adapter.rs @@ -16,7 +16,7 @@ // limitations under the License. use crate::*; -use sp_staking::{DelegationInterface, DelegationMigrator}; +use sp_staking::{Agent, DelegationInterface, DelegationMigrator, Delegator}; /// Types of stake strategies. /// @@ -32,6 +32,50 @@ pub enum StakeStrategyType { Delegate, } +/// A type that only belongs in context of a pool. +/// +/// Maps directly [`Agent`] account. +#[derive(Clone, Debug)] +pub struct Pool(T); +impl Into> for Pool { + fn into(self) -> Agent { + Agent::from(self.0) + } +} +impl From for Pool { + fn from(acc: T) -> Self { + Pool(acc) + } +} + +impl Pool { + pub fn get(self) -> T { + self.0 + } +} + +/// A type that only belongs in context of a pool member. +/// +/// Maps directly [`Delegator`] account. +#[derive(Clone, Debug)] +pub struct Member(T); +impl Into> for Member { + fn into(self) -> Delegator { + Delegator::from(self.0) + } +} +impl From for Member { + fn from(acc: T) -> Self { + Member(acc) + } +} + +impl Member { + pub fn get(self) -> T { + self.0 + } +} + /// An adapter trait that can support multiple staking strategies. /// /// Depending on which staking strategy we want to use, the staking logic can be slightly @@ -64,30 +108,30 @@ pub trait StakeStrategy { /// /// This is part of the pool balance that is not actively staked. That is, tokens that are /// in unbonding period or unbonded. - fn transferable_balance(pool_account: &Self::AccountId) -> Self::Balance; + fn transferable_balance(pool_account: Pool) -> Self::Balance; /// Total balance of the pool including amount that is actively staked. - fn total_balance(pool_account: &Self::AccountId) -> Self::Balance; + fn total_balance(pool_account: Pool) -> Option; /// Amount of tokens delegated by the member. - fn member_delegation_balance(member_account: &Self::AccountId) -> Self::Balance; + fn member_delegation_balance(member_account: Member) -> Option; /// See [`StakingInterface::active_stake`]. - fn active_stake(pool_account: &Self::AccountId) -> Self::Balance { - Self::CoreStaking::active_stake(pool_account).unwrap_or_default() + fn active_stake(pool_account: Pool) -> Self::Balance { + Self::CoreStaking::active_stake(&pool_account.0).unwrap_or_default() } /// See [`StakingInterface::total_stake`]. - fn total_stake(pool_account: &Self::AccountId) -> Self::Balance { - Self::CoreStaking::total_stake(pool_account).unwrap_or_default() + fn total_stake(pool_account: Pool) -> Self::Balance { + Self::CoreStaking::total_stake(&pool_account.0).unwrap_or_default() } /// Which strategy the pool account is using. /// /// This can be different from the [`Self::strategy_type`] of the adapter if the pool has not /// migrated to the new strategy yet. - fn pool_strategy(pool_account: &Self::AccountId) -> StakeStrategyType { - match Self::CoreStaking::is_virtual_staker(pool_account) { + fn pool_strategy(pool_account: Pool) -> StakeStrategyType { + match Self::CoreStaking::is_virtual_staker(&pool_account.0) { true => StakeStrategyType::Delegate, false => StakeStrategyType::Transfer, } @@ -95,55 +139,55 @@ pub trait StakeStrategy { /// See [`StakingInterface::nominate`]. fn nominate( - pool_account: &Self::AccountId, + pool_account: Pool, validators: Vec, ) -> DispatchResult { - Self::CoreStaking::nominate(pool_account, validators) + Self::CoreStaking::nominate(&pool_account.0, validators) } /// See [`StakingInterface::chill`]. - fn chill(pool_account: &Self::AccountId) -> DispatchResult { - Self::CoreStaking::chill(pool_account) + fn chill(pool_account: Pool) -> DispatchResult { + Self::CoreStaking::chill(&pool_account.0) } /// Pledge `amount` towards `pool_account` and update the pool bond. Also see /// [`StakingInterface::bond`]. fn pledge_bond( - who: &Self::AccountId, - pool_account: &Self::AccountId, + who: Member, + pool_account: Pool, reward_account: &Self::AccountId, amount: Self::Balance, bond_type: BondType, ) -> DispatchResult; /// See [`StakingInterface::unbond`]. - fn unbond(pool_account: &Self::AccountId, amount: Self::Balance) -> DispatchResult { - Self::CoreStaking::unbond(pool_account, amount) + fn unbond(pool_account: Pool, amount: Self::Balance) -> DispatchResult { + Self::CoreStaking::unbond(&pool_account.0, amount) } /// See [`StakingInterface::withdraw_unbonded`]. fn withdraw_unbonded( - pool_account: &Self::AccountId, + pool_account: Pool, num_slashing_spans: u32, ) -> Result { - Self::CoreStaking::withdraw_unbonded(pool_account.clone(), num_slashing_spans) + Self::CoreStaking::withdraw_unbonded(pool_account.0, num_slashing_spans) } /// Withdraw funds from pool account to member account. fn member_withdraw( - who: &Self::AccountId, - pool_account: &Self::AccountId, + who: Member, + pool_account: Pool, amount: Self::Balance, num_slashing_spans: u32, ) -> DispatchResult; /// Check if there is any pending slash for the pool. - fn has_pending_slash(pool_account: &Self::AccountId) -> bool; + fn pending_slash(pool_account: Pool) -> Self::Balance; /// Slash the member account with `amount` against pending slashes for the pool. fn member_slash( - who: &Self::AccountId, - pool_account: &Self::AccountId, + who: Member, + pool_account: Pool, amount: Self::Balance, maybe_reporter: Option, ) -> DispatchResult; @@ -153,7 +197,7 @@ pub trait StakeStrategy { /// This is useful for migrating a pool account from [`StakeStrategyType::Transfer`] to /// [`StakeStrategyType::Delegate`]. fn migrate_nominator_to_agent( - pool_account: &Self::AccountId, + pool_account: Pool, reward_account: &Self::AccountId, ) -> DispatchResult; @@ -166,15 +210,15 @@ pub trait StakeStrategy { /// Internally, the member funds that are locked in the pool account are transferred back and /// locked in the member account. fn migrate_delegation( - pool: &Self::AccountId, - delegator: &Self::AccountId, + pool: Pool, + delegator: Member, value: Self::Balance, ) -> DispatchResult; /// List of validators nominated by the pool account. #[cfg(feature = "runtime-benchmarks")] - fn nominations(pool_account: &Self::AccountId) -> Option> { - Self::CoreStaking::nominations(pool_account) + fn nominations(pool_account: Pool) -> Option> { + Self::CoreStaking::nominations(&pool_account.0) } /// Remove the pool account as agent. @@ -182,7 +226,7 @@ pub trait StakeStrategy { /// Useful for migrating pool account from a delegated agent to a direct nominator. Only used /// in tests and benchmarks. #[cfg(feature = "runtime-benchmarks")] - fn remove_as_agent(_pool: &Self::AccountId) { + fn remove_as_agent(_pool: Pool) { // noop by default } } @@ -209,22 +253,24 @@ impl, AccountId = T: StakeStrategyType::Transfer } - fn transferable_balance(pool_account: &Self::AccountId) -> BalanceOf { - T::Currency::balance(pool_account).saturating_sub(Self::active_stake(pool_account)) + fn transferable_balance(pool_account: Pool) -> BalanceOf { + T::Currency::balance(&pool_account.0).saturating_sub(Self::active_stake(pool_account)) } - fn total_balance(pool_account: &Self::AccountId) -> BalanceOf { - T::Currency::total_balance(pool_account) + fn total_balance(pool_account: Pool) -> Option> { + Some(T::Currency::total_balance(&pool_account.0)) } - fn member_delegation_balance(_member_account: &T::AccountId) -> Staking::Balance { - // for transfer stake, delegation balance is always zero. - Zero::zero() + fn member_delegation_balance( + _member_account: Member, + ) -> Option { + // for transfer stake, no delegation exists. + None } fn pledge_bond( - who: &T::AccountId, - pool_account: &Self::AccountId, + who: Member, + pool_account: Pool, reward_account: &Self::AccountId, amount: BalanceOf, bond_type: BondType, @@ -232,36 +278,36 @@ impl, AccountId = T: match bond_type { BondType::Create => { // first bond - T::Currency::transfer(who, pool_account, amount, Preservation::Expendable)?; - Staking::bond(pool_account, amount, &reward_account) + T::Currency::transfer(&who.0, &pool_account.0, amount, Preservation::Expendable)?; + Staking::bond(&pool_account.0, amount, &reward_account) }, BondType::Extra => { // additional bond - T::Currency::transfer(who, pool_account, amount, Preservation::Preserve)?; - Staking::bond_extra(pool_account, amount) + T::Currency::transfer(&who.0, &pool_account.0, amount, Preservation::Preserve)?; + Staking::bond_extra(&pool_account.0, amount) }, } } fn member_withdraw( - who: &T::AccountId, - pool_account: &Self::AccountId, + who: Member, + pool_account: Pool, amount: BalanceOf, _num_slashing_spans: u32, ) -> DispatchResult { - T::Currency::transfer(pool_account, &who, amount, Preservation::Expendable)?; + T::Currency::transfer(&pool_account.0, &who.0, amount, Preservation::Expendable)?; Ok(()) } - fn has_pending_slash(_: &Self::AccountId) -> bool { + fn pending_slash(_: Pool) -> Self::Balance { // for transfer stake strategy, slashing is greedy and never deferred. - false + Zero::zero() } fn member_slash( - _who: &T::AccountId, - _pool: &Self::AccountId, + _who: Member, + _pool: Pool, _amount: Staking::Balance, _maybe_reporter: Option, ) -> DispatchResult { @@ -269,15 +315,15 @@ impl, AccountId = T: } fn migrate_nominator_to_agent( - _pool: &Self::AccountId, + _pool: Pool, _reward_account: &Self::AccountId, ) -> DispatchResult { Err(Error::::Defensive(DefensiveError::DelegationUnsupported).into()) } fn migrate_delegation( - _pool: &Self::AccountId, - _delegator: &Self::AccountId, + _pool: Pool, + _delegator: Member, _value: Self::Balance, ) -> DispatchResult { Err(Error::::Defensive(DefensiveError::DelegationUnsupported).into()) @@ -314,21 +360,24 @@ impl< StakeStrategyType::Delegate } - fn transferable_balance(pool_account: &Self::AccountId) -> BalanceOf { - Delegation::agent_balance(pool_account).saturating_sub(Self::active_stake(pool_account)) + fn transferable_balance(pool_account: Pool) -> BalanceOf { + Delegation::agent_balance(pool_account.clone().into()) + // pool should always be an agent. + .defensive_unwrap_or_default() + .saturating_sub(Self::active_stake(pool_account)) } - fn total_balance(pool_account: &Self::AccountId) -> BalanceOf { - Delegation::agent_balance(pool_account) + fn total_balance(pool_account: Pool) -> Option> { + Delegation::agent_balance(pool_account.into()) } - fn member_delegation_balance(member_account: &T::AccountId) -> BalanceOf { - Delegation::delegator_balance(member_account) + fn member_delegation_balance(member_account: Member) -> Option> { + Delegation::delegator_balance(member_account.into()) } fn pledge_bond( - who: &T::AccountId, - pool_account: &Self::AccountId, + who: Member, + pool_account: Pool, reward_account: &Self::AccountId, amount: BalanceOf, bond_type: BondType, @@ -336,54 +385,54 @@ impl< match bond_type { BondType::Create => { // first delegation - Delegation::delegate(who, pool_account, reward_account, amount) + Delegation::delegate(who.into(), pool_account.into(), reward_account, amount) }, BondType::Extra => { // additional delegation - Delegation::delegate_extra(who, pool_account, amount) + Delegation::delegate_extra(who.into(), pool_account.into(), amount) }, } } fn member_withdraw( - who: &T::AccountId, - pool_account: &Self::AccountId, + who: Member, + pool_account: Pool, amount: BalanceOf, num_slashing_spans: u32, ) -> DispatchResult { - Delegation::withdraw_delegation(&who, pool_account, amount, num_slashing_spans) + Delegation::withdraw_delegation(who.into(), pool_account.into(), amount, num_slashing_spans) } - fn has_pending_slash(pool_account: &Self::AccountId) -> bool { - Delegation::has_pending_slash(pool_account) + fn pending_slash(pool_account: Pool) -> Self::Balance { + Delegation::pending_slash(pool_account.into()).defensive_unwrap_or_default() } fn member_slash( - who: &T::AccountId, - pool_account: &Self::AccountId, + who: Member, + pool_account: Pool, amount: BalanceOf, maybe_reporter: Option, ) -> DispatchResult { - Delegation::delegator_slash(pool_account, who, amount, maybe_reporter) + Delegation::delegator_slash(pool_account.into(), who.into(), amount, maybe_reporter) } fn migrate_nominator_to_agent( - pool: &Self::AccountId, + pool: Pool, reward_account: &Self::AccountId, ) -> DispatchResult { - Delegation::migrate_nominator_to_agent(pool, reward_account) + Delegation::migrate_nominator_to_agent(pool.into(), reward_account) } fn migrate_delegation( - pool: &Self::AccountId, - delegator: &Self::AccountId, + pool: Pool, + delegator: Member, value: Self::Balance, ) -> DispatchResult { - Delegation::migrate_delegation(pool, delegator, value) + Delegation::migrate_delegation(pool.into(), delegator.into(), value) } #[cfg(feature = "runtime-benchmarks")] - fn remove_as_agent(pool: &Self::AccountId) { - Delegation::drop_agent(pool) + fn remove_as_agent(pool: Pool) { + Delegation::drop_agent(pool.into()) } } diff --git a/substrate/frame/nomination-pools/src/lib.rs b/substrate/frame/nomination-pools/src/lib.rs index 816334c1a0840..2aaea04463661 100644 --- a/substrate/frame/nomination-pools/src/lib.rs +++ b/substrate/frame/nomination-pools/src/lib.rs @@ -351,7 +351,7 @@ #![cfg_attr(not(feature = "std"), no_std)] -use adapter::StakeStrategy; +use adapter::{Member, Pool, StakeStrategy}; use codec::Codec; use frame_support::{ defensive, defensive_assert, ensure, @@ -1007,7 +1007,7 @@ impl BondedPool { /// /// This is often used for bonding and issuing new funds into the pool. fn balance_to_point(&self, new_funds: BalanceOf) -> BalanceOf { - let bonded_balance = T::StakeAdapter::active_stake(&self.bonded_account()); + let bonded_balance = T::StakeAdapter::active_stake(Pool::from(self.bonded_account())); Pallet::::balance_to_point(bonded_balance, self.points, new_funds) } @@ -1015,7 +1015,7 @@ impl BondedPool { /// /// This is often used for unbonding. fn points_to_balance(&self, points: BalanceOf) -> BalanceOf { - let bonded_balance = T::StakeAdapter::active_stake(&self.bonded_account()); + let bonded_balance = T::StakeAdapter::active_stake(Pool::from(self.bonded_account())); Pallet::::point_to_balance(bonded_balance, self.points, points) } @@ -1125,7 +1125,7 @@ impl BondedPool { fn ok_to_be_open(&self) -> Result<(), DispatchError> { ensure!(!self.is_destroying(), Error::::CanNotChangeState); - let bonded_balance = T::StakeAdapter::active_stake(&self.bonded_account()); + let bonded_balance = T::StakeAdapter::active_stake(Pool::from(self.bonded_account())); ensure!(!bonded_balance.is_zero(), Error::::OverflowRisk); let points_to_balance_ratio_floor = self @@ -1259,8 +1259,8 @@ impl BondedPool { let points_issued = self.issue(amount); T::StakeAdapter::pledge_bond( - who, - &self.bonded_account(), + Member::from(who.clone()), + Pool::from(self.bonded_account()), &self.reward_account(), amount, ty, @@ -1940,12 +1940,10 @@ pub mod pallet { NothingToAdjust, /// No slash pending that can be applied to the member. NothingToSlash, - /// No delegation to migrate. - NoDelegationToMigrate, - /// The pool has already migrated to enable delegation. - PoolAlreadyMigrated, - /// The pool has not migrated yet to enable delegation. - PoolNotMigrated, + /// The pool or member delegation has already migrated to delegate stake. + AlreadyMigrated, + /// The pool or member delegation has not migrated yet to delegate stake. + NotMigrated, /// This call is not allowed in the current state of the pallet. NotSupported, } @@ -2148,7 +2146,7 @@ pub mod pallet { // Unbond in the actual underlying nominator. let unbonding_balance = bonded_pool.dissolve(unbonding_points); - T::StakeAdapter::unbond(&bonded_pool.bonded_account(), unbonding_balance)?; + T::StakeAdapter::unbond(Pool::from(bonded_pool.bonded_account()), unbonding_balance)?; // Note that we lazily create the unbonding pools here if they don't already exist let mut sub_pools = SubPoolsStorage::::get(member.pool_id) @@ -2211,7 +2209,10 @@ pub mod pallet { // For now we only allow a pool to withdraw unbonded if its not destroying. If the pool // is destroying then `withdraw_unbonded` can be used. ensure!(pool.state != PoolState::Destroying, Error::::NotDestroying); - T::StakeAdapter::withdraw_unbonded(&pool.bonded_account(), num_slashing_spans)?; + T::StakeAdapter::withdraw_unbonded( + Pool::from(pool.bonded_account()), + num_slashing_spans, + )?; Ok(()) } @@ -2285,7 +2286,7 @@ pub mod pallet { // Before calculating the `balance_to_unbond`, we call withdraw unbonded to ensure the // `transferable_balance` is correct. let stash_killed = T::StakeAdapter::withdraw_unbonded( - &bonded_pool.bonded_account(), + Pool::from(bonded_pool.bonded_account()), num_slashing_spans, )?; @@ -2334,13 +2335,15 @@ pub mod pallet { // don't exist. This check is also defensive in cases where the unbond pool does not // update its balance (e.g. a bug in the slashing hook.) We gracefully proceed in // order to ensure members can leave the pool and it can be destroyed. - .min(T::StakeAdapter::transferable_balance(&bonded_pool.bonded_account())); + .min(T::StakeAdapter::transferable_balance(Pool::from( + bonded_pool.bonded_account(), + ))); // this can fail if the pool uses `DelegateStake` strategy and the member delegation // is not claimed yet. See `Call::migrate_delegation()`. T::StakeAdapter::member_withdraw( - &member_account, - &bonded_pool.bonded_account(), + Member::from(member_account.clone()), + Pool::from(bonded_pool.bonded_account()), balance_to_unbond, num_slashing_spans, )?; @@ -2473,7 +2476,7 @@ pub mod pallet { Error::::MinimumBondNotMet ); - T::StakeAdapter::nominate(&bonded_pool.bonded_account(), validators) + T::StakeAdapter::nominate(Pool::from(bonded_pool.bonded_account()), validators) } /// Set a new state for the pool. @@ -2666,7 +2669,7 @@ pub mod pallet { ensure!(bonded_pool.can_nominate(&who), Error::::NotNominator); } - T::StakeAdapter::chill(&bonded_pool.bonded_account()) + T::StakeAdapter::chill(Pool::from(bonded_pool.bonded_account())) } /// `origin` bonds funds from `extra` for some pool member `member` into their respective @@ -2918,25 +2921,26 @@ pub mod pallet { // ensure pool is migrated. ensure!( - T::StakeAdapter::pool_strategy(&Self::generate_bonded_account(member.pool_id)) == - adapter::StakeStrategyType::Delegate, - Error::::PoolNotMigrated + T::StakeAdapter::pool_strategy(Pool::from(Self::generate_bonded_account( + member.pool_id + ))) == adapter::StakeStrategyType::Delegate, + Error::::NotMigrated ); 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::::NoDelegationToMigrate); + ensure!(pool_contribution > Zero::zero(), Error::::AlreadyMigrated); - let delegation = T::StakeAdapter::member_delegation_balance(&member_account); - // delegation can be claimed only once. - ensure!(delegation == Zero::zero(), Error::::NoDelegationToMigrate); + let delegation = + T::StakeAdapter::member_delegation_balance(Member::from(member_account.clone())); + // delegation should not exist. + ensure!(delegation.is_none(), Error::::AlreadyMigrated); - let diff = pool_contribution.defensive_saturating_sub(delegation); T::StakeAdapter::migrate_delegation( - &Pallet::::generate_bonded_account(member.pool_id), - &member_account, - diff, + Pool::from(Pallet::::generate_bonded_account(member.pool_id)), + Member::from(member_account), + pool_contribution, )?; // if successful, we refund the fee. @@ -2968,9 +2972,9 @@ pub mod pallet { // ensure pool exists. let bonded_pool = BondedPool::::get(pool_id).ok_or(Error::::PoolNotFound)?; ensure!( - T::StakeAdapter::pool_strategy(&bonded_pool.bonded_account()) == + T::StakeAdapter::pool_strategy(Pool::from(bonded_pool.bonded_account())) == adapter::StakeStrategyType::Transfer, - Error::::PoolAlreadyMigrated + Error::::AlreadyMigrated ); Self::migrate_to_delegate_stake(pool_id)?; @@ -3045,7 +3049,7 @@ impl Pallet { "bonded account of dissolving pool should have no consumers" ); defensive_assert!( - T::StakeAdapter::total_stake(&bonded_pool.bonded_account()) == Zero::zero(), + T::StakeAdapter::total_stake(Pool::from(bonded_pool.bonded_account())) == Zero::zero(), "dissolving pool should not have any stake in the staking pallet" ); @@ -3068,12 +3072,14 @@ impl Pallet { "could not transfer all amount to depositor while dissolving pool" ); defensive_assert!( - T::StakeAdapter::total_balance(&bonded_pool.bonded_account()) == Zero::zero(), + T::StakeAdapter::total_balance(Pool::from(bonded_pool.bonded_account())) + .unwrap_or_default() == + Zero::zero(), "dissolving pool should not have any balance" ); // NOTE: Defensively force set balance to zero. T::Currency::set_balance(&reward_account, Zero::zero()); - // With `DelegateStake` strategy, this won't do anything. + // NOTE: With `DelegateStake` strategy, this won't do anything. T::Currency::set_balance(&bonded_pool.bonded_account(), Zero::zero()); Self::deposit_event(Event::::Destroyed { pool_id: bonded_pool.id }); @@ -3090,7 +3096,7 @@ impl Pallet { fn migrate_to_delegate_stake(id: PoolId) -> DispatchResult { T::StakeAdapter::migrate_nominator_to_agent( - &Self::generate_bonded_account(id), + Pool::from(Self::generate_bonded_account(id)), &Self::generate_reward_account(id), ) } @@ -3468,31 +3474,59 @@ impl Pallet { member_account: &T::AccountId, reporter: Option, ) -> DispatchResult { - // calculate points to be slashed. - let member = - PoolMembers::::get(&member_account).ok_or(Error::::PoolMemberNotFound)?; + let member = PoolMembers::::get(member_account).ok_or(Error::::PoolMemberNotFound)?; - let pool_account = Pallet::::generate_bonded_account(member.pool_id); - ensure!(T::StakeAdapter::has_pending_slash(&pool_account), Error::::NothingToSlash); - - let unslashed_balance = T::StakeAdapter::member_delegation_balance(&member_account); - let slashed_balance = member.total_balance(); - defensive_assert!( - unslashed_balance >= slashed_balance, - "unslashed balance should always be greater or equal to the slashed" - ); + let pending_slash = + Self::member_pending_slash(Member::from(member_account.clone()), member.clone())?; // if nothing to slash, return error. - ensure!(unslashed_balance > slashed_balance, Error::::NothingToSlash); + ensure!(!pending_slash.is_zero(), Error::::NothingToSlash); T::StakeAdapter::member_slash( - &member_account, - &pool_account, - unslashed_balance.defensive_saturating_sub(slashed_balance), + Member::from(member_account.clone()), + Pool::from(Pallet::::generate_bonded_account(member.pool_id)), + pending_slash, reporter, ) } + /// Pending slash for a member. + /// + /// Takes the pool_member object corresponding to the `member_account`. + fn member_pending_slash( + member_account: Member, + pool_member: PoolMember, + ) -> Result, DispatchError> { + // only executed in tests: ensure the member account is correct. + debug_assert!( + PoolMembers::::get(member_account.clone().get()).expect("member must exist") == + pool_member + ); + + let pool_account = Pallet::::generate_bonded_account(pool_member.pool_id); + // if the pool doesn't have any pending slash, it implies the member also does not have any + // pending slash. + if T::StakeAdapter::pending_slash(Pool::from(pool_account.clone())).is_zero() { + return Ok(Zero::zero()) + } + + // this is their actual held balance that may or may not have been slashed. + let actual_balance = T::StakeAdapter::member_delegation_balance(member_account) + // no delegation implies the member delegation is not migrated yet to `DelegateStake`. + .ok_or(Error::::NotMigrated)?; + + // this is their balance in the pool + let expected_balance = pool_member.total_balance(); + + defensive_assert!( + actual_balance >= expected_balance, + "actual balance should always be greater or equal to the expected" + ); + + // return the amount to be slashed. + Ok(actual_balance.defensive_saturating_sub(expected_balance)) + } + /// Apply freeze on reward account to restrict it from going below ED. pub(crate) fn freeze_pool_deposit(reward_acc: &T::AccountId) -> DispatchResult { T::Currency::set_freeze( @@ -3656,7 +3690,7 @@ impl Pallet { pool is being destroyed and the depositor is the last member", ); - expected_tvl += T::StakeAdapter::total_stake(&bonded_pool.bonded_account()); + expected_tvl += T::StakeAdapter::total_stake(Pool::from(bonded_pool.bonded_account())); Ok(()) })?; @@ -3685,24 +3719,18 @@ impl Pallet { let subs = SubPoolsStorage::::get(pool_id).unwrap_or_default(); let sum_unbonding_balance = subs.sum_unbonding_balance(); - let bonded_balance = T::StakeAdapter::active_stake(&pool_account); - let total_balance = T::StakeAdapter::total_balance(&pool_account); - - // At the time when StakeAdapter is changed but migration is not yet done, the new - // adapter would return zero balance (as it is not an agent yet). We handle that by - // falling back to reading actual balance of the pool account. - let pool_balance = if total_balance.is_zero() { - T::Currency::total_balance(&pool_account) - } else { - total_balance - }; + let bonded_balance = T::StakeAdapter::active_stake(Pool::from(pool_account.clone())); + let total_balance = T::StakeAdapter::total_balance(Pool::from(pool_account.clone())) + // At the time when StakeAdapter is changed to `DelegateStake` but pool is not yet + // migrated, the total balance would be none. + .unwrap_or(T::Currency::total_balance(&pool_account)); assert!( - pool_balance >= bonded_balance + sum_unbonding_balance, - "faulty pool: {:?} / {:?}, pool_balance {:?} >= bonded_balance {:?} + sum_unbonding_balance {:?}", + total_balance >= bonded_balance + sum_unbonding_balance, + "faulty pool: {:?} / {:?}, total_balance {:?} >= bonded_balance {:?} + sum_unbonding_balance {:?}", pool_id, _pool, - pool_balance, + total_balance, bonded_balance, sum_unbonding_balance ); @@ -3799,12 +3827,75 @@ impl Pallet { pub fn api_balance_to_points(pool_id: PoolId, new_funds: BalanceOf) -> BalanceOf { if let Some(pool) = BondedPool::::get(pool_id) { let bonded_balance = - T::StakeAdapter::active_stake(&Self::generate_bonded_account(pool_id)); + T::StakeAdapter::active_stake(Pool::from(Self::generate_bonded_account(pool_id))); Pallet::::balance_to_point(bonded_balance, pool.points, new_funds) } else { Zero::zero() } } + + /// Returns the unapplied slash of the pool. + /// + /// Pending slash is only applicable with [`adapter::DelegateStake`] strategy. + pub fn api_pool_pending_slash(pool_id: PoolId) -> BalanceOf { + T::StakeAdapter::pending_slash(Pool::from(Self::generate_bonded_account(pool_id))) + } + + /// Returns the unapplied slash of a member. + /// + /// Pending slash is only applicable with [`adapter::DelegateStake`] strategy. + pub fn api_member_pending_slash(who: T::AccountId) -> BalanceOf { + PoolMembers::::get(who.clone()) + .map(|pool_member| { + Self::member_pending_slash(Member::from(who), pool_member).unwrap_or_default() + }) + .unwrap_or_default() + } + + /// Checks whether pool needs to be migrated to [`adapter::StakeStrategyType::Delegate`]. Only + /// applicable when the [`Config::StakeAdapter`] is [`adapter::DelegateStake`]. + /// + /// Useful to check this before calling [`Call::migrate_pool_to_delegate_stake`]. + pub fn api_pool_needs_delegate_migration(pool_id: PoolId) -> bool { + // if the `Delegate` strategy is not used in the pallet, then no migration required. + if T::StakeAdapter::strategy_type() != adapter::StakeStrategyType::Delegate { + 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 + } + + /// Checks whether member delegation needs to be migrated to + /// [`adapter::StakeStrategyType::Delegate`]. Only applicable when the [`Config::StakeAdapter`] + /// is [`adapter::DelegateStake`]. + /// + /// Useful to check this before calling [`Call::migrate_delegation`]. + pub fn api_member_needs_delegate_migration(who: T::AccountId) -> bool { + // if the `Delegate` strategy is not used in the pallet, then no migration required. + if T::StakeAdapter::strategy_type() != adapter::StakeStrategyType::Delegate { + return false + } + + PoolMembers::::get(who.clone()) + .map(|pool_member| { + if Self::api_pool_needs_delegate_migration(pool_member.pool_id) { + // the pool needs to be migrated before members can be migrated. + return false + } + + let member_balance = pool_member.total_balance(); + let delegated_balance = + T::StakeAdapter::member_delegation_balance(Member::from(who.clone())); + + // if the member has no delegation but has some balance in the pool, then it needs + // to be migrated. + delegated_balance.is_none() && !member_balance.is_zero() + }) + .unwrap_or_default() + } } impl sp_staking::OnStakingUpdate> for Pallet { diff --git a/substrate/frame/nomination-pools/src/migration.rs b/substrate/frame/nomination-pools/src/migration.rs index a3989559dfbb0..a9222ea53d75f 100644 --- a/substrate/frame/nomination-pools/src/migration.rs +++ b/substrate/frame/nomination-pools/src/migration.rs @@ -135,7 +135,8 @@ pub mod unversioned { let pool_acc = Pallet::::generate_bonded_account(id); // only migrate if the pool is in Transfer Strategy. - if T::StakeAdapter::pool_strategy(&pool_acc) == adapter::StakeStrategyType::Transfer + if T::StakeAdapter::pool_strategy(Pool::from(pool_acc)) == + adapter::StakeStrategyType::Transfer { let _ = Pallet::::migrate_to_delegate_stake(id).map_err(|err| { log!( @@ -178,14 +179,11 @@ pub mod unversioned { let mut pool_balances: Vec> = Vec::new(); BondedPools::::iter_keys().take(MaxPools::get() as usize).for_each(|id| { let pool_account = Pallet::::generate_bonded_account(id); - let current_strategy = T::StakeAdapter::pool_strategy(&pool_account); // we ensure migration is idempotent. - let pool_balance = if current_strategy == adapter::StakeStrategyType::Transfer { - T::Currency::total_balance(&pool_account) - } else { - T::StakeAdapter::total_balance(&pool_account) - }; + let pool_balance = T::StakeAdapter::total_balance(Pool::from(pool_account.clone())) + // we check actual account balance if pool has not migrated yet. + .unwrap_or(T::Currency::total_balance(&pool_account)); pool_balances.push(pool_balance); }); @@ -201,14 +199,16 @@ pub mod unversioned { BondedPools::::iter_keys().take(MaxPools::get() as usize).enumerate() { let pool_account = Pallet::::generate_bonded_account(id); - if T::StakeAdapter::pool_strategy(&pool_account) == + if T::StakeAdapter::pool_strategy(Pool::from(pool_account.clone())) == adapter::StakeStrategyType::Transfer { log!(error, "Pool {} failed to migrate", id,); return Err(TryRuntimeError::Other("Pool failed to migrate")); } - let actual_balance = T::StakeAdapter::total_balance(&pool_account); + let actual_balance = + T::StakeAdapter::total_balance(Pool::from(pool_account.clone())) + .expect("after migration, this should return a value"); let expected_balance = expected_pool_balances.get(index).unwrap(); if actual_balance != *expected_balance { @@ -1154,7 +1154,9 @@ mod helpers { pub(crate) fn calculate_tvl_by_total_stake() -> BalanceOf { BondedPools::::iter_keys() - .map(|id| T::StakeAdapter::total_stake(&Pallet::::generate_bonded_account(id))) + .map(|id| { + T::StakeAdapter::total_stake(Pool::from(Pallet::::generate_bonded_account(id))) + }) .reduce(|acc, total_balance| acc + total_balance) .unwrap_or_default() } diff --git a/substrate/frame/nomination-pools/src/tests.rs b/substrate/frame/nomination-pools/src/tests.rs index 8fc339c695bde..28063c2ecaecd 100644 --- a/substrate/frame/nomination-pools/src/tests.rs +++ b/substrate/frame/nomination-pools/src/tests.rs @@ -5021,6 +5021,10 @@ mod set_state { Error::::NotSupported ); + // pending slash api should return zero as well. + assert_eq!(Pools::api_pool_pending_slash(1), 0); + assert_eq!(Pools::api_member_pending_slash(10), 0); + // When assert_ok!(Pools::set_state(RuntimeOrigin::signed(11), 1, PoolState::Destroying)); // Then @@ -7518,12 +7522,14 @@ mod delegate_stake { ); // ensure pool 1 cannot be migrated. + assert!(!Pools::api_pool_needs_delegate_migration(1)); assert_noop!( Pools::migrate_pool_to_delegate_stake(RuntimeOrigin::signed(10), 1), Error::::NotSupported ); // members cannot be migrated either. + assert!(!Pools::api_member_needs_delegate_migration(10)); assert_noop!( Pools::migrate_delegation(RuntimeOrigin::signed(10), 11), Error::::NotSupported 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 d3235760ed23b..51f6470f90d02 100644 --- a/substrate/frame/nomination-pools/test-delegate-stake/src/lib.rs +++ b/substrate/frame/nomination-pools/test-delegate-stake/src/lib.rs @@ -35,6 +35,7 @@ use pallet_staking::{ use pallet_delegated_staking::{Error as DelegatedStakingError, Event as DelegatedStakingEvent}; use sp_runtime::{bounded_btree_map, traits::Zero}; +use sp_staking::Agent; #[test] fn pool_lifecycle_e2e() { @@ -666,6 +667,10 @@ fn pool_slash_proportional() { // Apply a slash that happened in era 100. This is typically applied with a delay. // Of the total 100, 50 is slashed. assert_eq!(BondedPools::::get(1).unwrap().points, 40); + + // no pending slash yet. + assert_eq!(Pools::api_pool_pending_slash(1), 0); + pallet_staking::slashing::do_slash::( &POOL1_BONDED, 50, @@ -674,6 +679,9 @@ fn pool_slash_proportional() { 100, ); + // Pools api returns correct slash amount. + assert_eq!(Pools::api_pool_pending_slash(1), 50); + assert_eq!( staking_events_since_last_call(), vec![StakingEvent::Slashed { staker: POOL1_BONDED, amount: 50 }] @@ -695,10 +703,14 @@ fn pool_slash_proportional() { assert_eq!(PoolMembers::::get(21).unwrap().total_balance(), 7); // But their actual balance is still unslashed. assert_eq!(Balances::total_balance_on_hold(&21), bond); + // 21 has pending slash + assert_eq!(Pools::api_member_pending_slash(21), bond - 7); // apply slash permissionlessly. assert_ok!(Pools::apply_slash(RuntimeOrigin::signed(10), 21)); // member balance is slashed. assert_eq!(Balances::total_balance_on_hold(&21), 7); + // 21 has no pending slash anymore + assert_eq!(Pools::api_member_pending_slash(21), 0); assert_eq!( delegated_staking_events_since_last_call(), @@ -977,6 +989,7 @@ fn pool_migration_e2e() { ); // with `TransferStake`, we can't migrate. + assert!(!Pools::api_pool_needs_delegate_migration(1)); assert_noop!( Pools::migrate_pool_to_delegate_stake(RuntimeOrigin::signed(10), 1), PoolsError::::NotSupported @@ -986,22 +999,26 @@ 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::::PoolNotMigrated + PoolsError::::NotMigrated ); // migrate the pool. + assert!(Pools::api_pool_needs_delegate_migration(1)); assert_ok!(Pools::migrate_pool_to_delegate_stake(RuntimeOrigin::signed(10), 1)); // migrate again does not work. + assert!(!Pools::api_pool_needs_delegate_migration(1)); assert_noop!( Pools::migrate_pool_to_delegate_stake(RuntimeOrigin::signed(10), 1), - PoolsError::::PoolAlreadyMigrated + PoolsError::::AlreadyMigrated ); // unclaimed delegations to the pool are stored in this account. - let proxy_delegator_1 = DelegatedStaking::generate_proxy_delegator(POOL1_BONDED); + let proxy_delegator_1 = + DelegatedStaking::generate_proxy_delegator(Agent::from(POOL1_BONDED)).get(); assert_eq!( delegated_staking_events_since_last_call(), @@ -1027,6 +1044,7 @@ fn pool_migration_e2e() { assert_eq!(Balances::total_balance_on_hold(&20), 0); // migrate delegation for 20. This is permissionless and can be called by anyone. + assert!(Pools::api_member_needs_delegate_migration(20)); assert_ok!(Pools::migrate_delegation(RuntimeOrigin::signed(10), 20)); // tokens moved to 20's account and held there. @@ -1071,6 +1089,7 @@ fn pool_migration_e2e() { assert_eq!(Balances::total_balance_on_hold(&21), 0); // migrate delegation for 21. + assert!(Pools::api_member_needs_delegate_migration(21)); assert_ok!(Pools::migrate_delegation(RuntimeOrigin::signed(10), 21)); // tokens moved to 21's account and held there. @@ -1098,8 +1117,16 @@ fn pool_migration_e2e() { assert_eq!(Balances::total_balance_on_hold(&22), 0); // migrate delegation for 22. + assert!(Pools::api_member_needs_delegate_migration(22)); assert_ok!(Pools::migrate_delegation(RuntimeOrigin::signed(10), 22)); + // cannot migrate a pool member again. + assert!(!Pools::api_member_needs_delegate_migration(22)); + assert_noop!( + Pools::migrate_delegation(RuntimeOrigin::signed(10), 22), + PoolsError::::AlreadyMigrated + ); + // 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_on_hold(&22), 10); diff --git a/substrate/frame/nomination-pools/test-delegate-stake/src/mock.rs b/substrate/frame/nomination-pools/test-delegate-stake/src/mock.rs index 1c0a0166fd9a9..5018232635981 100644 --- a/substrate/frame/nomination-pools/test-delegate-stake/src/mock.rs +++ b/substrate/frame/nomination-pools/test-delegate-stake/src/mock.rs @@ -24,7 +24,10 @@ use frame_support::{ PalletId, }; use frame_system::EnsureRoot; -use pallet_nomination_pools::{adapter::StakeStrategyType, BondType}; +use pallet_nomination_pools::{ + adapter::{Member, Pool, StakeStrategyType}, + BondType, +}; use sp_runtime::{ traits::{Convert, IdentityLookup}, BuildStorage, FixedU128, Perbill, @@ -190,21 +193,21 @@ impl pallet_nomination_pools::adapter::StakeStrategy for MockAdapter { } DelegateStake::strategy_type() } - fn transferable_balance(pool_account: &Self::AccountId) -> Self::Balance { + fn transferable_balance(pool_account: Pool) -> Self::Balance { if LegacyAdapter::get() { return TransferStake::transferable_balance(pool_account) } DelegateStake::transferable_balance(pool_account) } - fn total_balance(pool_account: &Self::AccountId) -> Self::Balance { + fn total_balance(pool_account: Pool) -> Option { if LegacyAdapter::get() { return TransferStake::total_balance(pool_account) } DelegateStake::total_balance(pool_account) } - fn member_delegation_balance(member_account: &Self::AccountId) -> Self::Balance { + fn member_delegation_balance(member_account: Member) -> Option { if LegacyAdapter::get() { return TransferStake::member_delegation_balance(member_account) } @@ -212,8 +215,8 @@ impl pallet_nomination_pools::adapter::StakeStrategy for MockAdapter { } fn pledge_bond( - who: &Self::AccountId, - pool_account: &Self::AccountId, + who: Member, + pool_account: Pool, reward_account: &Self::AccountId, amount: Self::Balance, bond_type: BondType, @@ -225,8 +228,8 @@ impl pallet_nomination_pools::adapter::StakeStrategy for MockAdapter { } fn member_withdraw( - who: &Self::AccountId, - pool_account: &Self::AccountId, + who: Member, + pool_account: Pool, amount: Self::Balance, num_slashing_spans: u32, ) -> DispatchResult { @@ -236,16 +239,16 @@ impl pallet_nomination_pools::adapter::StakeStrategy for MockAdapter { DelegateStake::member_withdraw(who, pool_account, amount, num_slashing_spans) } - fn has_pending_slash(pool_account: &Self::AccountId) -> bool { + fn pending_slash(pool_account: Pool) -> Self::Balance { if LegacyAdapter::get() { - return TransferStake::has_pending_slash(pool_account) + return TransferStake::pending_slash(pool_account) } - DelegateStake::has_pending_slash(pool_account) + DelegateStake::pending_slash(pool_account) } fn member_slash( - who: &Self::AccountId, - pool_account: &Self::AccountId, + who: Member, + pool_account: Pool, amount: Self::Balance, maybe_reporter: Option, ) -> DispatchResult { @@ -256,7 +259,7 @@ impl pallet_nomination_pools::adapter::StakeStrategy for MockAdapter { } fn migrate_nominator_to_agent( - agent: &Self::AccountId, + agent: Pool, reward_account: &Self::AccountId, ) -> DispatchResult { if LegacyAdapter::get() { @@ -266,8 +269,8 @@ impl pallet_nomination_pools::adapter::StakeStrategy for MockAdapter { } fn migrate_delegation( - agent: &Self::AccountId, - delegator: &Self::AccountId, + agent: Pool, + delegator: Member, value: Self::Balance, ) -> DispatchResult { if LegacyAdapter::get() { diff --git a/substrate/primitives/staking/src/lib.rs b/substrate/primitives/staking/src/lib.rs index 28a61cd433139..c1cf7f2778ff6 100644 --- a/substrate/primitives/staking/src/lib.rs +++ b/substrate/primitives/staking/src/lib.rs @@ -463,17 +463,48 @@ pub struct PagedExposureMetadata { pub page_count: Page, } -/// Trait to provide delegation functionality for stakers. +/// A type that belongs only in the context of an `Agent`. /// -/// Introduces two new terms to the staking system: -/// - `Delegator`: An account that delegates funds to an `Agent`. -/// - `Agent`: An account that receives delegated funds from `Delegators`. It can then use these -/// funds to participate in the staking system. It can never use its own funds to stake. They -/// (virtually bond)[`StakingUnchecked::virtual_bond`] into the staking system and can also be -/// termed as `Virtual Nominators`. +/// `Agent` is someone that manages delegated funds from [`Delegator`] accounts. It can +/// then use these funds to participate in the staking system. It can never use its own funds to +/// stake. They instead (virtually bond)[`StakingUnchecked::virtual_bond`] into the staking system +/// and are also called `Virtual Stakers`. /// -/// The `Agent` is responsible for managing rewards and slashing for all the `Delegators` that +/// The `Agent` is also responsible for managing rewards and slashing for all the `Delegators` that /// have delegated funds to it. +#[derive(Clone, Debug)] +pub struct Agent(T); +impl From for Agent { + fn from(acc: T) -> Self { + Agent(acc) + } +} + +impl Agent { + pub fn get(self) -> T { + self.0 + } +} + +/// A type that belongs only in the context of a `Delegator`. +/// +/// `Delegator` is someone that delegates funds to an `Agent`, allowing them to pool funds +/// along with other delegators and participate in the staking system. +#[derive(Clone, Debug)] +pub struct Delegator(T); +impl From for Delegator { + fn from(acc: T) -> Self { + Delegator(acc) + } +} + +impl Delegator { + pub fn get(self) -> T { + self.0 + } +} + +/// Trait to provide delegation functionality for stakers. pub trait DelegationInterface { /// Balance type used by the staking system. type Balance: Sub @@ -489,20 +520,20 @@ pub trait DelegationInterface { /// AccountId type used by the staking system. type AccountId: Clone + core::fmt::Debug; - /// Effective balance of the `Agent` account. + /// Returns effective balance of the `Agent` account. `None` if not an `Agent`. /// - /// This takes into account any pending slashes to `Agent`. - fn agent_balance(agent: &Self::AccountId) -> Self::Balance; + /// This takes into account any pending slashes to `Agent` against the delegated balance. + fn agent_balance(agent: Agent) -> Option; - /// Returns the total amount of funds delegated by a `delegator`. - fn delegator_balance(delegator: &Self::AccountId) -> Self::Balance; + /// Returns the total amount of funds delegated. `None` if not a `Delegator`. + fn delegator_balance(delegator: Delegator) -> Option; /// Delegate funds to `Agent`. /// /// Only used for the initial delegation. Use [`Self::delegate_extra`] to add more delegation. fn delegate( - delegator: &Self::AccountId, - agent: &Self::AccountId, + delegator: Delegator, + agent: Agent, reward_account: &Self::AccountId, amount: Self::Balance, ) -> DispatchResult; @@ -511,8 +542,8 @@ pub trait DelegationInterface { /// /// If this is the first delegation, use [`Self::delegate`] instead. fn delegate_extra( - delegator: &Self::AccountId, - agent: &Self::AccountId, + delegator: Delegator, + agent: Agent, amount: Self::Balance, ) -> DispatchResult; @@ -521,25 +552,25 @@ pub trait DelegationInterface { /// If there are `Agent` funds upto `amount` available to withdraw, then those funds would /// be released to the `delegator` fn withdraw_delegation( - delegator: &Self::AccountId, - agent: &Self::AccountId, + delegator: Delegator, + agent: Agent, amount: Self::Balance, num_slashing_spans: u32, ) -> DispatchResult; - /// Returns true if there are pending slashes posted to the `Agent` account. + /// Returns pending slashes posted to the `Agent` account. None if not an `Agent`. /// /// Slashes to `Agent` account are not immediate and are applied lazily. Since `Agent` /// has an unbounded number of delegators, immediate slashing is not possible. - fn has_pending_slash(agent: &Self::AccountId) -> bool; + fn pending_slash(agent: Agent) -> Option; /// Apply a pending slash to an `Agent` by slashing `value` from `delegator`. /// /// A reporter may be provided (if one exists) in order for the implementor to reward them, /// if applicable. fn delegator_slash( - agent: &Self::AccountId, - delegator: &Self::AccountId, + agent: Agent, + delegator: Delegator, value: Self::Balance, maybe_reporter: Option, ) -> DispatchResult; @@ -567,7 +598,7 @@ pub trait DelegationMigrator { /// The implementation should ensure the `Nominator` account funds are moved to an escrow /// from which `Agents` can later release funds to its `Delegators`. fn migrate_nominator_to_agent( - agent: &Self::AccountId, + agent: Agent, reward_account: &Self::AccountId, ) -> DispatchResult; @@ -576,8 +607,8 @@ pub trait DelegationMigrator { /// When a direct `Nominator` migrates to `Agent`, the funds are kept in escrow. This function /// allows the `Agent` to release the funds to the `delegator`. fn migrate_delegation( - agent: &Self::AccountId, - delegator: &Self::AccountId, + agent: Agent, + delegator: Delegator, value: Self::Balance, ) -> DispatchResult; @@ -585,7 +616,7 @@ pub trait DelegationMigrator { /// /// Also removed from [`StakingUnchecked`] as a Virtual Staker. Useful for testing. #[cfg(feature = "runtime-benchmarks")] - fn drop_agent(agent: &Self::AccountId); + fn drop_agent(agent: Agent); } sp_core::generate_feature_enabled_macro!(runtime_benchmarks_enabled, feature = "runtime-benchmarks", $);