Skip to content

Commit

Permalink
Allow privileged virtual bond in Staking pallet (#3889)
Browse files Browse the repository at this point in the history
This is the first PR in preparation for
#454.

## Follow ups:
- #3904.
- #3905.

Overall changes are documented here (lot more visual 😍):
https://hackmd.io/@ak0n/454-np-governance

[Maybe followup](#4217)
with migration of storage item `VirtualStakers` as a bool or enum in
`Ledger`.

## Context
We want to achieve a way for a user (`Delegator`) to delegate their
funds to another account (`Agent`). Delegate implies the funds are
locked in delegator account itself. Agent can act on behalf of delegator
to stake directly on Staking pallet.

The delegation feature is added to Staking via another pallet
`delegated-staking` worked on
[here](#3904).

## Introduces:
### StakingUnchecked Trait
As the name implies, this trait allows unchecked (non-locked) mutation
of staking ledger. These apis are only meant to be used by other pallets
in the runtime and should not be exposed directly to user code path.
Also related: #3888.

### Virtual Bond
Allows other pallets to stake via staking pallet while managing the
locks on these accounts themselves. Introduces another storage
`VirtualStakers` that whitelist these accounts.

We also restrict virtual stakers to set reward account as themselves.
Since the account has no locks, we cannot support compounding of
rewards. Conservatively, we require them to set a separate account
different from the staker. Since these are code managed, it should be
easy for another pallet to redistribute reward and rebond them.

### Slashes
Since there is no actual lock maintained by staking-pallet for virtual
stakers, this pallet does not apply any slashes. It is then important
for pallets managing virtual stakers to listen to slashing events and
apply necessary slashes.
  • Loading branch information
Ank4n authored Apr 20, 2024
1 parent 4eabe5e commit e504c41
Show file tree
Hide file tree
Showing 9 changed files with 513 additions and 72 deletions.
14 changes: 14 additions & 0 deletions prdoc/pr_3889.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# 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: Allow privileged virtual bond into pallet Staking

doc:
- audience: Runtime Dev
description: |
Introduces a new low level API to allow privileged virtual bond into pallet Staking. This allows other pallets
to stake funds into staking pallet while managing the fund lock and unlocking process themselves.

crates:
- name: pallet-staking

8 changes: 8 additions & 0 deletions substrate/frame/nomination-pools/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,10 @@ impl sp_staking::StakingInterface for StakingMock {
Ok(())
}

fn update_payee(_stash: &Self::AccountId, _reward_acc: &Self::AccountId) -> DispatchResult {
unimplemented!("method currently not used in testing")
}

fn chill(_: &Self::AccountId) -> sp_runtime::DispatchResult {
Ok(())
}
Expand Down Expand Up @@ -223,6 +227,10 @@ impl sp_staking::StakingInterface for StakingMock {
fn max_exposure_page_size() -> sp_staking::Page {
unimplemented!("method currently not used in testing")
}

fn slash_reward_fraction() -> Perbill {
unimplemented!("method currently not used in testing")
}
}

#[derive_impl(frame_system::config_preludes::TestDefaultConfig)]
Expand Down
43 changes: 29 additions & 14 deletions substrate/frame/staking/src/ledger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,14 @@

use frame_support::{
defensive, ensure,
traits::{Defensive, LockableCurrency, WithdrawReasons},
traits::{Defensive, LockableCurrency},
};
use sp_staking::StakingAccount;
use sp_std::prelude::*;

use crate::{
BalanceOf, Bonded, Config, Error, Ledger, Payee, RewardDestination, StakingLedger, STAKING_ID,
BalanceOf, Bonded, Config, Error, Ledger, Pallet, Payee, RewardDestination, StakingLedger,
VirtualStakers, STAKING_ID,
};

#[cfg(any(feature = "runtime-benchmarks", test))]
Expand Down Expand Up @@ -187,7 +188,17 @@ impl<T: Config> StakingLedger<T> {
return Err(Error::<T>::NotStash)
}

T::Currency::set_lock(STAKING_ID, &self.stash, self.total, WithdrawReasons::all());
// We skip locking virtual stakers.
if !Pallet::<T>::is_virtual_staker(&self.stash) {
// for direct stakers, update lock on stash based on ledger.
T::Currency::set_lock(
STAKING_ID,
&self.stash,
self.total,
frame_support::traits::WithdrawReasons::all(),
);
}

Ledger::<T>::insert(
&self.controller().ok_or_else(|| {
defensive!("update called on a ledger that is not bonded.");
Expand All @@ -204,22 +215,22 @@ impl<T: Config> StakingLedger<T> {
/// It sets the reward preferences for the bonded stash.
pub(crate) fn bond(self, payee: RewardDestination<T::AccountId>) -> Result<(), Error<T>> {
if <Bonded<T>>::contains_key(&self.stash) {
Err(Error::<T>::AlreadyBonded)
} else {
<Payee<T>>::insert(&self.stash, payee);
<Bonded<T>>::insert(&self.stash, &self.stash);
self.update()
return Err(Error::<T>::AlreadyBonded)
}

<Payee<T>>::insert(&self.stash, payee);
<Bonded<T>>::insert(&self.stash, &self.stash);
self.update()
}

/// Sets the ledger Payee.
pub(crate) fn set_payee(self, payee: RewardDestination<T::AccountId>) -> Result<(), Error<T>> {
if !<Bonded<T>>::contains_key(&self.stash) {
Err(Error::<T>::NotStash)
} else {
<Payee<T>>::insert(&self.stash, payee);
Ok(())
return Err(Error::<T>::NotStash)
}

<Payee<T>>::insert(&self.stash, payee);
Ok(())
}

/// Sets the ledger controller to its stash.
Expand Down Expand Up @@ -252,12 +263,16 @@ impl<T: Config> StakingLedger<T> {
let controller = <Bonded<T>>::get(stash).ok_or(Error::<T>::NotStash)?;

<Ledger<T>>::get(&controller).ok_or(Error::<T>::NotController).map(|ledger| {
T::Currency::remove_lock(STAKING_ID, &ledger.stash);
Ledger::<T>::remove(controller);

<Bonded<T>>::remove(&stash);
<Payee<T>>::remove(&stash);

// kill virtual staker if it exists.
if <VirtualStakers<T>>::take(&stash).is_none() {
// if not virtual staker, clear locks.
T::Currency::remove_lock(STAKING_ID, &ledger.stash);
}

Ok(())
})?
}
Expand Down
23 changes: 21 additions & 2 deletions substrate/frame/staking/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,17 +249,21 @@ parameter_types! {
pub static LedgerSlashPerEra:
(BalanceOf<Test>, BTreeMap<EraIndex, BalanceOf<Test>>) =
(Zero::zero(), BTreeMap::new());
pub static SlashObserver: BTreeMap<AccountId, BalanceOf<Test>> = BTreeMap::new();
}

pub struct EventListenerMock;
impl OnStakingUpdate<AccountId, Balance> for EventListenerMock {
fn on_slash(
_pool_account: &AccountId,
pool_account: &AccountId,
slashed_bonded: Balance,
slashed_chunks: &BTreeMap<EraIndex, Balance>,
_total_slashed: Balance,
total_slashed: Balance,
) {
LedgerSlashPerEra::set((slashed_bonded, slashed_chunks.clone()));
SlashObserver::mutate(|map| {
map.insert(*pool_account, map.get(pool_account).unwrap_or(&0) + total_slashed)
});
}
}

Expand Down Expand Up @@ -598,6 +602,21 @@ pub(crate) fn bond_nominator(who: AccountId, val: Balance, target: Vec<AccountId
assert_ok!(Staking::nominate(RuntimeOrigin::signed(who), target));
}

pub(crate) fn bond_virtual_nominator(
who: AccountId,
payee: AccountId,
val: Balance,
target: Vec<AccountId>,
) {
// In a real scenario, `who` is a keyless account managed by another pallet which provides for
// it.
System::inc_providers(&who);

// Bond who virtually.
assert_ok!(<Staking as sp_staking::StakingUnchecked>::virtual_bond(&who, val, &payee));
assert_ok!(Staking::nominate(RuntimeOrigin::signed(who), target));
}

/// Progress to the given block, triggering session and era changes as we progress.
///
/// This will finalize the previous block, initialize up to the given block, essentially simulating
Expand Down
155 changes: 147 additions & 8 deletions substrate/frame/staking/src/pallet/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,18 @@ use frame_support::{
pallet_prelude::*,
traits::{
Currency, Defensive, DefensiveSaturating, EstimateNextNewSession, Get, Imbalance,
InspectLockableCurrency, Len, OnUnbalanced, TryCollect, UnixTime,
InspectLockableCurrency, Len, LockableCurrency, OnUnbalanced, TryCollect, UnixTime,
},
weights::Weight,
};
use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin};
use pallet_session::historical;
use sp_runtime::{
traits::{Bounded, Convert, One, SaturatedConversion, Saturating, StaticLookup, Zero},
Perbill, Percent,
traits::{
Bounded, CheckedAdd, CheckedSub, Convert, One, SaturatedConversion, Saturating,
StaticLookup, Zero,
},
ArithmeticError, Perbill, Percent,
};
use sp_staking::{
currency_to_vote::CurrencyToVote,
Expand Down Expand Up @@ -149,6 +152,39 @@ impl<T: Config> Pallet<T> {
Self::slashable_balance_of_vote_weight(who, issuance)
}

pub(super) fn do_bond_extra(stash: &T::AccountId, additional: BalanceOf<T>) -> DispatchResult {
let mut ledger = Self::ledger(StakingAccount::Stash(stash.clone()))?;

// for virtual stakers, we don't need to check the balance. Since they are only accessed
// via low level apis, we can assume that the caller has done the due diligence.
let extra = if Self::is_virtual_staker(stash) {
additional
} else {
// additional amount or actual balance of stash whichever is lower.
additional.min(
T::Currency::free_balance(stash)
.checked_sub(&ledger.total)
.ok_or(ArithmeticError::Overflow)?,
)
};

ledger.total = ledger.total.checked_add(&extra).ok_or(ArithmeticError::Overflow)?;
ledger.active = ledger.active.checked_add(&extra).ok_or(ArithmeticError::Overflow)?;
// last check: the new active amount of ledger must be more than ED.
ensure!(ledger.active >= T::Currency::minimum_balance(), Error::<T>::InsufficientBond);

// NOTE: ledger must be updated prior to calling `Self::weight_of`.
ledger.update()?;
// update this staker in the sorted list, if they exist in it.
if T::VoterList::contains(stash) {
let _ = T::VoterList::on_update(&stash, Self::weight_of(stash)).defensive();
}

Self::deposit_event(Event::<T>::Bonded { stash: stash.clone(), amount: extra });

Ok(())
}

pub(super) fn do_withdraw_unbonded(
controller: &T::AccountId,
num_slashing_spans: u32,
Expand Down Expand Up @@ -1132,6 +1168,11 @@ impl<T: Config> Pallet<T> {
) -> Exposure<T::AccountId, BalanceOf<T>> {
EraInfo::<T>::get_full_exposure(era, account)
}

/// Whether `who` is a virtual staker whose funds are managed by another pallet.
pub(crate) fn is_virtual_staker(who: &T::AccountId) -> bool {
VirtualStakers::<T>::contains_key(who)
}
}

impl<T: Config> Pallet<T> {
Expand Down Expand Up @@ -1748,6 +1789,23 @@ impl<T: Config> StakingInterface for Pallet<T> {
.map(|_| ())
}

fn update_payee(stash: &Self::AccountId, reward_acc: &Self::AccountId) -> DispatchResult {
// Since virtual stakers are not allowed to compound their rewards as this pallet does not
// manage their locks, we do not allow reward account to be set same as stash. For
// external pallets that manage the virtual bond, they can claim rewards and re-bond them.
ensure!(
!Self::is_virtual_staker(stash) || stash != reward_acc,
Error::<T>::RewardDestinationRestricted
);

// since controller is deprecated and this function is never used for old ledgers with
// distinct controllers, we can safely assume that stash is the controller.
Self::set_payee(
RawOrigin::Signed(stash.clone()).into(),
RewardDestination::Account(reward_acc.clone()),
)
}

fn chill(who: &Self::AccountId) -> DispatchResult {
// defensive-only: any account bonded via this interface has the stash set as the
// controller, but we have to be sure. Same comment anywhere else that we read this.
Expand Down Expand Up @@ -1832,6 +1890,10 @@ impl<T: Config> StakingInterface for Pallet<T> {
}
}

fn slash_reward_fraction() -> Perbill {
SlashRewardFraction::<T>::get()
}

sp_staking::runtime_benchmarks_enabled! {
fn nominations(who: &Self::AccountId) -> Option<Vec<T::AccountId>> {
Nominators::<T>::get(who).map(|n| n.targets.into_inner())
Expand Down Expand Up @@ -1860,6 +1922,55 @@ impl<T: Config> StakingInterface for Pallet<T> {
}
}

impl<T: Config> sp_staking::StakingUnchecked for Pallet<T> {
fn migrate_to_virtual_staker(who: &Self::AccountId) {
T::Currency::remove_lock(crate::STAKING_ID, who);
VirtualStakers::<T>::insert(who, ());
}

/// Virtually bonds `keyless_who` to `payee` with `value`.
///
/// The payee must not be the same as the `keyless_who`.
fn virtual_bond(
keyless_who: &Self::AccountId,
value: Self::Balance,
payee: &Self::AccountId,
) -> DispatchResult {
if StakingLedger::<T>::is_bonded(StakingAccount::Stash(keyless_who.clone())) {
return Err(Error::<T>::AlreadyBonded.into())
}

// check if payee not same as who.
ensure!(keyless_who != payee, Error::<T>::RewardDestinationRestricted);

// mark this pallet as consumer of `who`.
frame_system::Pallet::<T>::inc_consumers(&keyless_who).map_err(|_| Error::<T>::BadState)?;

// mark who as a virtual staker.
VirtualStakers::<T>::insert(keyless_who, ());

Self::deposit_event(Event::<T>::Bonded { stash: keyless_who.clone(), amount: value });
let ledger = StakingLedger::<T>::new(keyless_who.clone(), value);

ledger.bond(RewardDestination::Account(payee.clone()))?;

Ok(())
}

#[cfg(feature = "runtime-benchmarks")]
fn migrate_to_direct_staker(who: &Self::AccountId) {
assert!(VirtualStakers::<T>::contains_key(who));
let ledger = StakingLedger::<T>::get(Stash(who.clone())).unwrap();
T::Currency::set_lock(
crate::STAKING_ID,
who,
ledger.total,
frame_support::traits::WithdrawReasons::all(),
);
VirtualStakers::<T>::remove(who);
}
}

#[cfg(any(test, feature = "try-runtime"))]
impl<T: Config> Pallet<T> {
pub(crate) fn do_try_state(_: BlockNumberFor<T>) -> Result<(), TryRuntimeError> {
Expand Down Expand Up @@ -1980,16 +2091,44 @@ impl<T: Config> Pallet<T> {
/// Invariants:
/// * Stake consistency: ledger.total == ledger.active + sum(ledger.unlocking).
/// * The ledger's controller and stash matches the associated `Bonded` tuple.
/// * Staking locked funds for every bonded stash should be the same as its ledger's total.
/// * Staking locked funds for every bonded stash (non virtual stakers) should be the same as
/// its ledger's total.
/// * For virtual stakers, locked funds should be zero and payee should be non-stash account.
/// * Staking ledger and bond are not corrupted.
fn check_ledgers() -> Result<(), TryRuntimeError> {
Bonded::<T>::iter()
.map(|(stash, ctrl)| {
// ensure locks consistency.
ensure!(
Self::inspect_bond_state(&stash) == Ok(LedgerIntegrityState::Ok),
"bond, ledger and/or staking lock inconsistent for a bonded stash."
);
if VirtualStakers::<T>::contains_key(stash.clone()) {
ensure!(
T::Currency::balance_locked(crate::STAKING_ID, &stash) == Zero::zero(),
"virtual stakers should not have any locked balance"
);
ensure!(
<Bonded<T>>::get(stash.clone()).unwrap() == stash.clone(),
"stash and controller should be same"
);
ensure!(
Ledger::<T>::get(stash.clone()).unwrap().stash == stash,
"ledger corrupted for virtual staker"
);
let reward_destination = <Payee<T>>::get(stash.clone()).unwrap();
if let RewardDestination::Account(payee) = reward_destination {
ensure!(
payee != stash.clone(),
"reward destination should not be same as stash for virtual staker"
);
} else {
return Err(DispatchError::Other(
"reward destination must be of account variant for virtual staker",
));
}
} else {
ensure!(
Self::inspect_bond_state(&stash) == Ok(LedgerIntegrityState::Ok),
"bond, ledger and/or staking lock inconsistent for a bonded stash."
);
}

// ensure ledger consistency.
Self::ensure_ledger_consistent(ctrl)
Expand Down
Loading

0 comments on commit e504c41

Please sign in to comment.