Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Nomination Pool to support multiple staking strategies #3905

Merged
merged 314 commits into from
May 22, 2024

Conversation

Ank4n
Copy link
Contributor

@Ank4n Ank4n commented Mar 30, 2024

Third and final PR in the set, closes #454.

Original PR: #2680

Precursors:

Follow up issues/improvements

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

Summary of various roles 🤯

Pallet Staking

Nominator: An account that directly stakes on pallet-staking and nominates a set of validators.
Stakers: Common term for nominators and validators.
Virtual Stakers: Same as stakers, but they are keyless accounts and their locks are managed by a pallet external to pallet-staking.

Pallet Delegated Staking

Agent: An account that receives delegation from other accounts (delegators) and stakes on their behalf. They are also Virtual Stakers in pallet-staking where pallet-delegated-staking manages its locks.
Delegator: An account that delegates some funds to an agent.

Pallet Nomination Pools

Pool account: Keyless account of a pool where funds are pooled. Members pledge their funds towards the pools. These are going to become Agent accounts in pallet-delegated-staking.
Pool Members: They are individual members of the pool who contributed funds to it. They are also Delegator in pallet-delegated-staking.

Changes

Multiple Stake strategies

TransferStake: The current nomination pool logic can be considered a staking strategy where delegators transfer funds to pool and stake. In this scenario, funds are locked in pool account, and users lose the control of their funds.

DelegateStake: With this PR, we introduce a new staking strategy where individual delegators delegate fund to pool. Delegate implies funds are locked in delegator account itself. Important thing to note is, pool does not have funds of its own, but it has authorization from its members to use these funds for staking.

We extract out all the interaction of pool with staking interface into a new trait StakeStrategy. This is the logic that varies between the above two staking strategies. We use the trait StakeStrategy to implement above two strategies: TransferStake and DelegateStake.

NominationPool

Consumes an implementation of StakeStrategy instead of StakingInterface. I have renamed it from Staking to StakeAdapter to clarify the difference from the earlier used trait.

To enable delegation based staking in pool, Nomination pool can be configured as:

type StakeAdapter = pallet_nomination_pools::adapter::DelegateStake<Self, DelegatedStaking>;

Note that with the following configuration, the changes in the PR are no-op.

type StakeAdapter = pallet_nomination_pools::adapter::TransferStake<Self, Staking>;

Deployment roadmap

Plan to enable this only in Westend. In production runtimes, we can keep pool to use TransferStake which will be no functional change.

Once we have a full audit, we can enable this in Kusama followed by Polkadot.

TODO

  • Runtime level (Westend) migration for existing nomination pools.
  • Permissionless call/ pallet::tasks for claiming delegator funds.
  • Add/update benches.
  • Migration tests.
  • Storage flag to mark DelegateStake migration and integrity checks to not allow TransferStake for migrated runtimes.

@Ank4n Ank4n changed the base branch from master to ankan/02-pallet-delegated-staking March 30, 2024 02:29
@Ank4n Ank4n force-pushed the ankan/03-np-delegation-integration branch from 929cd0f to 2b74eaf Compare March 31, 2024 18:04
@Ank4n Ank4n force-pushed the ankan/02-pallet-delegated-staking branch from cca4319 to b550031 Compare March 31, 2024 19:28
@Ank4n Ank4n force-pushed the ankan/03-np-delegation-integration branch from 2b74eaf to cebea1a Compare March 31, 2024 19:31
@Ank4n Ank4n added T1-FRAME This PR/Issue is related to core FRAME, the framework. T2-pallets This PR/Issue is related to a particular pallet. labels Mar 31, 2024
@Ank4n Ank4n marked this pull request as ready for review April 1, 2024 09:56
@Ank4n Ank4n requested a review from a team as a code owner April 1, 2024 09:56
Copy link
Contributor

@georgepisaltu georgepisaltu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good. The delegate staking tests need to switch to the new interface to become relevant, but it should be fairly easy as the testing logic should be the same, maybe a couple of extra corner cases here and there. Will re-review once that's done.

substrate/frame/nomination-pools/src/lib.rs Show resolved Hide resolved
Comment on lines 554 to 559
let maybe_pool = BondedPool::<T>::get(self.pool_id);
if maybe_pool.is_none() {
defensive!("pool should exist; qed");
return Zero::zero();
}
let pool = maybe_pool.expect("checked pool is not none; qed");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the suggestion above it could just be:

Suggested change
let maybe_pool = BondedPool::<T>::get(self.pool_id);
if maybe_pool.is_none() {
defensive!("pool should exist; qed");
return Zero::zero();
}
let pool = maybe_pool.expect("checked pool is not none; qed");
let pool = BondedPool::<T>::get(self.pool_id);

#[pallet::call_index(5)]
#[pallet::weight(
// fixme(ank4n): fix weight taking into account potential slashing.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Option 1 is to have this weight function not take into account any slashing and only consume the weight later if necessary. We'd need a bench function for Self::do_apply_slash.

Option 2 is to have a new weight function that assumes worst case and then at the end refund any weight if necessary

#[pallet::weight(
   cmp::max(
      T::WeightInfo::withdraw_unbonded_regular(*num_slashing_spans),
      T::WeightInfo::withdraw_unbonded_with_slash(*num_slashing_spans)
   )
)]

substrate/frame/nomination-pools/src/lib.rs Show resolved Hide resolved
@Ank4n Ank4n added this pull request to the merge queue May 22, 2024
Merged via the queue into master with commit 8949856 May 22, 2024
147 of 150 checks passed
@Ank4n Ank4n deleted the ankan/03-np-delegation-integration branch May 22, 2024 20:00
github-merge-queue bot pushed a commit that referenced this pull request Jun 3, 2024
## Runtime Apis
Introduces the following runtime apis to facilitate dapps and wallets in
integrating with the `DelegateStake` functionalities of the pools
(related: #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 <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Andrei Sandu <[email protected]>
Signed-off-by: Adrian Catangiu <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: divdeploy <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: hongkuang <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: gemini132 <[email protected]>
Co-authored-by: Matteo Muraca <[email protected]>
Co-authored-by: Liam Aharon <[email protected]>
Co-authored-by: Kian Paimani <[email protected]>
Co-authored-by: Alexandru Gheorghe <[email protected]>
Co-authored-by: Alessandro Siniscalchi <[email protected]>
Co-authored-by: Andrei Sandu <[email protected]>
Co-authored-by: Ross Bulat <[email protected]>
Co-authored-by: Serban Iorga <[email protected]>
Co-authored-by: s0me0ne-unkn0wn <[email protected]>
Co-authored-by: Sam Johnson <[email protected]>
Co-authored-by: Adrian Catangiu <[email protected]>
Co-authored-by: Javier Viola <[email protected]>
Co-authored-by: Alexandru Vasile <[email protected]>
Co-authored-by: Niklas Adolfsson <[email protected]>
Co-authored-by: Dastan <[email protected]>
Co-authored-by: Clara van Staden <[email protected]>
Co-authored-by: Ron <[email protected]>
Co-authored-by: Vincent Geddes <[email protected]>
Co-authored-by: Svyatoslav Nikolsky <[email protected]>
Co-authored-by: Michal Kucharczyk <[email protected]>
Co-authored-by: Dino Pačandi <[email protected]>
Co-authored-by: Andrei Eres <[email protected]>
Co-authored-by: Alin Dima <[email protected]>
Co-authored-by: Andrei Sandu <[email protected]>
Co-authored-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: Branislav Kontur <[email protected]>
Co-authored-by: Sebastian Kunert <[email protected]>
Co-authored-by: gupnik <[email protected]>
Co-authored-by: Vladimir Istyufeev <[email protected]>
Co-authored-by: Lulu <[email protected]>
Co-authored-by: Juan Girini <[email protected]>
Co-authored-by: Francisco Aguirre <[email protected]>
Co-authored-by: Dónal Murray <[email protected]>
Co-authored-by: Shawn Tabrizi <[email protected]>
Co-authored-by: Kutsal Kaan Bilgin <[email protected]>
Co-authored-by: Ermal Kaleci <[email protected]>
Co-authored-by: ordian <[email protected]>
Co-authored-by: divdeploy <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Sergej Sakac <[email protected]>
Co-authored-by: Squirrel <[email protected]>
Co-authored-by: HongKuang <[email protected]>
Co-authored-by: Tsvetomir Dimitrov <[email protected]>
Co-authored-by: Egor_P <[email protected]>
Co-authored-by: Aaro Altonen <[email protected]>
Co-authored-by: Dmitry Markin <[email protected]>
Co-authored-by: Alexandru Vasile <[email protected]>
Co-authored-by: Léa Narzis <[email protected]>
Co-authored-by: Gonçalo Pestana <[email protected]>
Co-authored-by: georgepisaltu <[email protected]>
Co-authored-by: command-bot <>
Co-authored-by: PG Herveou <[email protected]>
Co-authored-by: jimwfs <[email protected]>
Co-authored-by: jimwfs <[email protected]>
Co-authored-by: polka.dom <[email protected]>
hitchhooker pushed a commit to ibp-network/polkadot-sdk that referenced this pull request Jun 5, 2024
This is the second PR in preparation for
paritytech#454.

## Also see
- **Precursor** paritytech#3889.
- **Follow up** paritytech#3905.

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

## Changes
### Delegation Interface
Provides delegation primitives for staking. 

Introduces two new roles:
- Agent: These are accounts who receive delegation from other accounts
(delegators) and stakes on behalf of them. The funds are held in
delegator accounts.
- Delegator: Accounts who delegate their funds to an agent authorising
them to use it for staking.

Supports
- A way for delegators to add or withdraw delegation to an agent.
- A way for an agent to slash a delegator during a slashing event.

### Pallet Delegated Staking
- Implements `DelegationInterface`.
- Lazy slashing: Any slashes to an Agent is posted in a ledger but not
immediately slashed. The agent can call
`DelegationInterface::delegator_slash` to slash the member and clear the
corresponding slash from its ledger.
- Consumes `StakingInterface` to provide `CoreStaking` features. In
reality, this will be `pallet-staking`.
- Ensures bookkeeping for agent and delegator are correct but leaves the
management of reward and slash logic upto the consumer of this pallet.
- While it does not expose any calls yet, it is written with the intent
of exposing these primitives via extrinsics.

## TODO
- [x] Improve unit tests in the pallet.
- [x] Separate slash reward perbill for rewarding the slash reporters?
- [x] Review if we should add more events.

---------

Co-authored-by: Kian Paimani <[email protected]>
Co-authored-by: Gonçalo Pestana <[email protected]>
Co-authored-by: georgepisaltu <[email protected]>
hitchhooker pushed a commit to ibp-network/polkadot-sdk that referenced this pull request Jun 5, 2024
…tytech#3905)

Third and final PR in the set, closes
paritytech#454.

Original PR: paritytech#2680

## Precursors:
- paritytech#3889.
- paritytech#3904.

## Follow up issues/improvements
- paritytech#4404

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

## Summary of various roles 🤯
### Pallet Staking
**Nominator**: An account that directly stakes on `pallet-staking` and
nominates a set of validators.
**Stakers**: Common term for nominators and validators.
Virtual Stakers: Same as stakers, but they are keyless accounts and
their locks are managed by a pallet external to `pallet-staking`.

### Pallet Delegated Staking
**Agent**: An account that receives delegation from other accounts
(delegators) and stakes on their behalf. They are also Virtual Stakers
in `pallet-staking` where `pallet-delegated-staking` manages its locks.
**Delegator**: An account that delegates some funds to an agent.

### Pallet Nomination Pools
**Pool account**: Keyless account of a pool where funds are pooled.
Members pledge their funds towards the pools. These are going to become
`Agent` accounts in `pallet-delegated-staking`.
**Pool Members**: They are individual members of the pool who
contributed funds to it. They are also `Delegator` in
`pallet-delegated-staking`.

## Changes
### Multiple Stake strategies

**TransferStake**: The current nomination pool logic can be considered a
staking strategy where delegators transfer funds to pool and stake. In
this scenario, funds are locked in pool account, and users lose the
control of their funds.

**DelegateStake**: With this PR, we introduce a new staking strategy
where individual delegators delegate fund to pool. `Delegate` implies
funds are locked in delegator account itself. Important thing to note
is, pool does not have funds of its own, but it has authorization from
its members to use these funds for staking.

We extract out all the interaction of pool with staking interface into a
new trait `StakeStrategy`. This is the logic that varies between the
above two staking strategies. We use the trait `StakeStrategy` to
implement above two strategies: `TransferStake` and `DelegateStake`.

### NominationPool
Consumes an implementation of `StakeStrategy` instead of
`StakingInterface`. I have renamed it from `Staking` to `StakeAdapter`
to clarify the difference from the earlier used trait.

To enable delegation based staking in pool, Nomination pool can be
configured as:
```
type StakeAdapter = pallet_nomination_pools::adapter::DelegateStake<Self, DelegatedStaking>;
```

Note that with the following configuration, the changes in the PR are
no-op.
```
type StakeAdapter = pallet_nomination_pools::adapter::TransferStake<Self, Staking>;
```

## Deployment roadmap
Plan to enable this only in Westend. In production runtimes, we can keep
pool to use `TransferStake` which will be no functional change.

Once we have a full audit, we can enable this in Kusama followed by
Polkadot.

## TODO
- [x] Runtime level (Westend) migration for existing nomination pools.
- [x] Permissionless call/ pallet::tasks for claiming delegator funds.
- [x] Add/update benches.
- [x] Migration tests.
- [x] Storage flag to mark `DelegateStake` migration and integrity
checks to not allow `TransferStake` for migrated runtimes.

---------

Signed-off-by: Matteo Muraca <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Andrei Sandu <[email protected]>
Signed-off-by: Adrian Catangiu <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: divdeploy <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: hongkuang <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: gemini132 <[email protected]>
Co-authored-by: Matteo Muraca <[email protected]>
Co-authored-by: Liam Aharon <[email protected]>
Co-authored-by: Kian Paimani <[email protected]>
Co-authored-by: Alexandru Gheorghe <[email protected]>
Co-authored-by: Alessandro Siniscalchi <[email protected]>
Co-authored-by: Andrei Sandu <[email protected]>
Co-authored-by: Ross Bulat <[email protected]>
Co-authored-by: Serban Iorga <[email protected]>
Co-authored-by: s0me0ne-unkn0wn <[email protected]>
Co-authored-by: Sam Johnson <[email protected]>
Co-authored-by: Adrian Catangiu <[email protected]>
Co-authored-by: Javier Viola <[email protected]>
Co-authored-by: Alexandru Vasile <[email protected]>
Co-authored-by: Niklas Adolfsson <[email protected]>
Co-authored-by: Dastan <[email protected]>
Co-authored-by: Clara van Staden <[email protected]>
Co-authored-by: Ron <[email protected]>
Co-authored-by: Vincent Geddes <[email protected]>
Co-authored-by: Svyatoslav Nikolsky <[email protected]>
Co-authored-by: Michal Kucharczyk <[email protected]>
Co-authored-by: Dino Pačandi <[email protected]>
Co-authored-by: Andrei Eres <[email protected]>
Co-authored-by: Alin Dima <[email protected]>
Co-authored-by: Andrei Sandu <[email protected]>
Co-authored-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: Branislav Kontur <[email protected]>
Co-authored-by: Sebastian Kunert <[email protected]>
Co-authored-by: gupnik <[email protected]>
Co-authored-by: Vladimir Istyufeev <[email protected]>
Co-authored-by: Lulu <[email protected]>
Co-authored-by: Juan Girini <[email protected]>
Co-authored-by: Francisco Aguirre <[email protected]>
Co-authored-by: Dónal Murray <[email protected]>
Co-authored-by: Shawn Tabrizi <[email protected]>
Co-authored-by: Kutsal Kaan Bilgin <[email protected]>
Co-authored-by: Ermal Kaleci <[email protected]>
Co-authored-by: ordian <[email protected]>
Co-authored-by: divdeploy <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Sergej Sakac <[email protected]>
Co-authored-by: Squirrel <[email protected]>
Co-authored-by: HongKuang <[email protected]>
Co-authored-by: Tsvetomir Dimitrov <[email protected]>
Co-authored-by: Egor_P <[email protected]>
Co-authored-by: Aaro Altonen <[email protected]>
Co-authored-by: Dmitry Markin <[email protected]>
Co-authored-by: Alexandru Vasile <[email protected]>
Co-authored-by: Léa Narzis <[email protected]>
Co-authored-by: Gonçalo Pestana <[email protected]>
Co-authored-by: georgepisaltu <[email protected]>
Co-authored-by: command-bot <>
Co-authored-by: PG Herveou <[email protected]>
Co-authored-by: jimwfs <[email protected]>
Co-authored-by: jimwfs <[email protected]>
Co-authored-by: polka.dom <[email protected]>
hitchhooker pushed a commit to ibp-network/polkadot-sdk that referenced this pull request Jun 5, 2024
## Runtime Apis
Introduces the following runtime apis to facilitate dapps and wallets in
integrating with the `DelegateStake` functionalities of the pools
(related: paritytech#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 <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Andrei Sandu <[email protected]>
Signed-off-by: Adrian Catangiu <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: divdeploy <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: hongkuang <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: gemini132 <[email protected]>
Co-authored-by: Matteo Muraca <[email protected]>
Co-authored-by: Liam Aharon <[email protected]>
Co-authored-by: Kian Paimani <[email protected]>
Co-authored-by: Alexandru Gheorghe <[email protected]>
Co-authored-by: Alessandro Siniscalchi <[email protected]>
Co-authored-by: Andrei Sandu <[email protected]>
Co-authored-by: Ross Bulat <[email protected]>
Co-authored-by: Serban Iorga <[email protected]>
Co-authored-by: s0me0ne-unkn0wn <[email protected]>
Co-authored-by: Sam Johnson <[email protected]>
Co-authored-by: Adrian Catangiu <[email protected]>
Co-authored-by: Javier Viola <[email protected]>
Co-authored-by: Alexandru Vasile <[email protected]>
Co-authored-by: Niklas Adolfsson <[email protected]>
Co-authored-by: Dastan <[email protected]>
Co-authored-by: Clara van Staden <[email protected]>
Co-authored-by: Ron <[email protected]>
Co-authored-by: Vincent Geddes <[email protected]>
Co-authored-by: Svyatoslav Nikolsky <[email protected]>
Co-authored-by: Michal Kucharczyk <[email protected]>
Co-authored-by: Dino Pačandi <[email protected]>
Co-authored-by: Andrei Eres <[email protected]>
Co-authored-by: Alin Dima <[email protected]>
Co-authored-by: Andrei Sandu <[email protected]>
Co-authored-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: Branislav Kontur <[email protected]>
Co-authored-by: Sebastian Kunert <[email protected]>
Co-authored-by: gupnik <[email protected]>
Co-authored-by: Vladimir Istyufeev <[email protected]>
Co-authored-by: Lulu <[email protected]>
Co-authored-by: Juan Girini <[email protected]>
Co-authored-by: Francisco Aguirre <[email protected]>
Co-authored-by: Dónal Murray <[email protected]>
Co-authored-by: Shawn Tabrizi <[email protected]>
Co-authored-by: Kutsal Kaan Bilgin <[email protected]>
Co-authored-by: Ermal Kaleci <[email protected]>
Co-authored-by: ordian <[email protected]>
Co-authored-by: divdeploy <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Sergej Sakac <[email protected]>
Co-authored-by: Squirrel <[email protected]>
Co-authored-by: HongKuang <[email protected]>
Co-authored-by: Tsvetomir Dimitrov <[email protected]>
Co-authored-by: Egor_P <[email protected]>
Co-authored-by: Aaro Altonen <[email protected]>
Co-authored-by: Dmitry Markin <[email protected]>
Co-authored-by: Alexandru Vasile <[email protected]>
Co-authored-by: Léa Narzis <[email protected]>
Co-authored-by: Gonçalo Pestana <[email protected]>
Co-authored-by: georgepisaltu <[email protected]>
Co-authored-by: command-bot <>
Co-authored-by: PG Herveou <[email protected]>
Co-authored-by: jimwfs <[email protected]>
Co-authored-by: jimwfs <[email protected]>
Co-authored-by: polka.dom <[email protected]>
@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/polkadot-digest-13-jun-2024/8598/1

@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/any-update-on-voting-while-staking-in-a-pool/8589/3

liuchengxu pushed a commit to liuchengxu/polkadot-sdk that referenced this pull request Jun 19, 2024
This is the second PR in preparation for
paritytech#454.

## Also see
- **Precursor** paritytech#3889.
- **Follow up** paritytech#3905.

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

## Changes
### Delegation Interface
Provides delegation primitives for staking. 

Introduces two new roles:
- Agent: These are accounts who receive delegation from other accounts
(delegators) and stakes on behalf of them. The funds are held in
delegator accounts.
- Delegator: Accounts who delegate their funds to an agent authorising
them to use it for staking.

Supports
- A way for delegators to add or withdraw delegation to an agent.
- A way for an agent to slash a delegator during a slashing event.

### Pallet Delegated Staking
- Implements `DelegationInterface`.
- Lazy slashing: Any slashes to an Agent is posted in a ledger but not
immediately slashed. The agent can call
`DelegationInterface::delegator_slash` to slash the member and clear the
corresponding slash from its ledger.
- Consumes `StakingInterface` to provide `CoreStaking` features. In
reality, this will be `pallet-staking`.
- Ensures bookkeeping for agent and delegator are correct but leaves the
management of reward and slash logic upto the consumer of this pallet.
- While it does not expose any calls yet, it is written with the intent
of exposing these primitives via extrinsics.

## TODO
- [x] Improve unit tests in the pallet.
- [x] Separate slash reward perbill for rewarding the slash reporters?
- [x] Review if we should add more events.

---------

Co-authored-by: Kian Paimani <[email protected]>
Co-authored-by: Gonçalo Pestana <[email protected]>
Co-authored-by: georgepisaltu <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Jul 3, 2024
Related: #4804.
Fixes the try state error in Westend:
https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6564522.
Passes here:
https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6580393

## Context
Currently in Kusama and Polkadot, an account can do both, directly
stake, and join a pool.

With the migration of pools to `DelegateStake` (See
#3905), the funds of pool
members are locked in a different way than for direct stakers.
- Pool member funds uses `holds`.
- `pallet-staking` uses deprecated locks (analogous to freeze) which can
overlap with holds.

An existing delegator can stake directly since pallet-staking only uses
free balance. But once an account becomes staker, we cannot allow them
to be delegator as this risks an account to use already staked (frozen)
funds in pools.

When an account gets into a situation where it is participating in both
pools and staking, it would no longer would be able to add any extra
bond to the pool but they can still withdraw funds.

## Changes
- Add test for the above scenario.
- Removes the assumption that a delegator cannot be a staker.
TomaszWaszczyk pushed a commit to TomaszWaszczyk/polkadot-sdk that referenced this pull request Jul 7, 2024
…#4904)

Related: paritytech#4804.
Fixes the try state error in Westend:
https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6564522.
Passes here:
https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6580393

## Context
Currently in Kusama and Polkadot, an account can do both, directly
stake, and join a pool.

With the migration of pools to `DelegateStake` (See
paritytech#3905), the funds of pool
members are locked in a different way than for direct stakers.
- Pool member funds uses `holds`.
- `pallet-staking` uses deprecated locks (analogous to freeze) which can
overlap with holds.

An existing delegator can stake directly since pallet-staking only uses
free balance. But once an account becomes staker, we cannot allow them
to be delegator as this risks an account to use already staked (frozen)
funds in pools.

When an account gets into a situation where it is participating in both
pools and staking, it would no longer would be able to add any extra
bond to the pool but they can still withdraw funds.

## Changes
- Add test for the above scenario.
- Removes the assumption that a delegator cannot be a staker.
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
Related to paritytech#3905.

Since virtual stakers does not have a real balance, they should not be
allowed to be reaped.

The proper reaping for agents slashed will be done in a separate PR.
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
This is the second PR in preparation for
paritytech#454.

## Also see
- **Precursor** paritytech#3889.
- **Follow up** paritytech#3905.

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

## Changes
### Delegation Interface
Provides delegation primitives for staking. 

Introduces two new roles:
- Agent: These are accounts who receive delegation from other accounts
(delegators) and stakes on behalf of them. The funds are held in
delegator accounts.
- Delegator: Accounts who delegate their funds to an agent authorising
them to use it for staking.

Supports
- A way for delegators to add or withdraw delegation to an agent.
- A way for an agent to slash a delegator during a slashing event.

### Pallet Delegated Staking
- Implements `DelegationInterface`.
- Lazy slashing: Any slashes to an Agent is posted in a ledger but not
immediately slashed. The agent can call
`DelegationInterface::delegator_slash` to slash the member and clear the
corresponding slash from its ledger.
- Consumes `StakingInterface` to provide `CoreStaking` features. In
reality, this will be `pallet-staking`.
- Ensures bookkeeping for agent and delegator are correct but leaves the
management of reward and slash logic upto the consumer of this pallet.
- While it does not expose any calls yet, it is written with the intent
of exposing these primitives via extrinsics.

## TODO
- [x] Improve unit tests in the pallet.
- [x] Separate slash reward perbill for rewarding the slash reporters?
- [x] Review if we should add more events.

---------

Co-authored-by: Kian Paimani <[email protected]>
Co-authored-by: Gonçalo Pestana <[email protected]>
Co-authored-by: georgepisaltu <[email protected]>
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
…tytech#3905)

Third and final PR in the set, closes
paritytech#454.

Original PR: paritytech#2680

## Precursors:
- paritytech#3889.
- paritytech#3904.

## Follow up issues/improvements
- paritytech#4404

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

## Summary of various roles 🤯
### Pallet Staking
**Nominator**: An account that directly stakes on `pallet-staking` and
nominates a set of validators.
**Stakers**: Common term for nominators and validators.
Virtual Stakers: Same as stakers, but they are keyless accounts and
their locks are managed by a pallet external to `pallet-staking`.

### Pallet Delegated Staking
**Agent**: An account that receives delegation from other accounts
(delegators) and stakes on their behalf. They are also Virtual Stakers
in `pallet-staking` where `pallet-delegated-staking` manages its locks.
**Delegator**: An account that delegates some funds to an agent.

### Pallet Nomination Pools
**Pool account**: Keyless account of a pool where funds are pooled.
Members pledge their funds towards the pools. These are going to become
`Agent` accounts in `pallet-delegated-staking`.
**Pool Members**: They are individual members of the pool who
contributed funds to it. They are also `Delegator` in
`pallet-delegated-staking`.

## Changes
### Multiple Stake strategies

**TransferStake**: The current nomination pool logic can be considered a
staking strategy where delegators transfer funds to pool and stake. In
this scenario, funds are locked in pool account, and users lose the
control of their funds.

**DelegateStake**: With this PR, we introduce a new staking strategy
where individual delegators delegate fund to pool. `Delegate` implies
funds are locked in delegator account itself. Important thing to note
is, pool does not have funds of its own, but it has authorization from
its members to use these funds for staking.

We extract out all the interaction of pool with staking interface into a
new trait `StakeStrategy`. This is the logic that varies between the
above two staking strategies. We use the trait `StakeStrategy` to
implement above two strategies: `TransferStake` and `DelegateStake`.

### NominationPool
Consumes an implementation of `StakeStrategy` instead of
`StakingInterface`. I have renamed it from `Staking` to `StakeAdapter`
to clarify the difference from the earlier used trait.

To enable delegation based staking in pool, Nomination pool can be
configured as:
```
type StakeAdapter = pallet_nomination_pools::adapter::DelegateStake<Self, DelegatedStaking>;
```

Note that with the following configuration, the changes in the PR are
no-op.
```
type StakeAdapter = pallet_nomination_pools::adapter::TransferStake<Self, Staking>;
```

## Deployment roadmap
Plan to enable this only in Westend. In production runtimes, we can keep
pool to use `TransferStake` which will be no functional change.

Once we have a full audit, we can enable this in Kusama followed by
Polkadot.

## TODO
- [x] Runtime level (Westend) migration for existing nomination pools.
- [x] Permissionless call/ pallet::tasks for claiming delegator funds.
- [x] Add/update benches.
- [x] Migration tests.
- [x] Storage flag to mark `DelegateStake` migration and integrity
checks to not allow `TransferStake` for migrated runtimes.

---------

Signed-off-by: Matteo Muraca <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Andrei Sandu <[email protected]>
Signed-off-by: Adrian Catangiu <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: divdeploy <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: hongkuang <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: gemini132 <[email protected]>
Co-authored-by: Matteo Muraca <[email protected]>
Co-authored-by: Liam Aharon <[email protected]>
Co-authored-by: Kian Paimani <[email protected]>
Co-authored-by: Alexandru Gheorghe <[email protected]>
Co-authored-by: Alessandro Siniscalchi <[email protected]>
Co-authored-by: Andrei Sandu <[email protected]>
Co-authored-by: Ross Bulat <[email protected]>
Co-authored-by: Serban Iorga <[email protected]>
Co-authored-by: s0me0ne-unkn0wn <[email protected]>
Co-authored-by: Sam Johnson <[email protected]>
Co-authored-by: Adrian Catangiu <[email protected]>
Co-authored-by: Javier Viola <[email protected]>
Co-authored-by: Alexandru Vasile <[email protected]>
Co-authored-by: Niklas Adolfsson <[email protected]>
Co-authored-by: Dastan <[email protected]>
Co-authored-by: Clara van Staden <[email protected]>
Co-authored-by: Ron <[email protected]>
Co-authored-by: Vincent Geddes <[email protected]>
Co-authored-by: Svyatoslav Nikolsky <[email protected]>
Co-authored-by: Michal Kucharczyk <[email protected]>
Co-authored-by: Dino Pačandi <[email protected]>
Co-authored-by: Andrei Eres <[email protected]>
Co-authored-by: Alin Dima <[email protected]>
Co-authored-by: Andrei Sandu <[email protected]>
Co-authored-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: Branislav Kontur <[email protected]>
Co-authored-by: Sebastian Kunert <[email protected]>
Co-authored-by: gupnik <[email protected]>
Co-authored-by: Vladimir Istyufeev <[email protected]>
Co-authored-by: Lulu <[email protected]>
Co-authored-by: Juan Girini <[email protected]>
Co-authored-by: Francisco Aguirre <[email protected]>
Co-authored-by: Dónal Murray <[email protected]>
Co-authored-by: Shawn Tabrizi <[email protected]>
Co-authored-by: Kutsal Kaan Bilgin <[email protected]>
Co-authored-by: Ermal Kaleci <[email protected]>
Co-authored-by: ordian <[email protected]>
Co-authored-by: divdeploy <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Sergej Sakac <[email protected]>
Co-authored-by: Squirrel <[email protected]>
Co-authored-by: HongKuang <[email protected]>
Co-authored-by: Tsvetomir Dimitrov <[email protected]>
Co-authored-by: Egor_P <[email protected]>
Co-authored-by: Aaro Altonen <[email protected]>
Co-authored-by: Dmitry Markin <[email protected]>
Co-authored-by: Alexandru Vasile <[email protected]>
Co-authored-by: Léa Narzis <[email protected]>
Co-authored-by: Gonçalo Pestana <[email protected]>
Co-authored-by: georgepisaltu <[email protected]>
Co-authored-by: command-bot <>
Co-authored-by: PG Herveou <[email protected]>
Co-authored-by: jimwfs <[email protected]>
Co-authored-by: jimwfs <[email protected]>
Co-authored-by: polka.dom <[email protected]>
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
## Runtime Apis
Introduces the following runtime apis to facilitate dapps and wallets in
integrating with the `DelegateStake` functionalities of the pools
(related: paritytech#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 <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Andrei Sandu <[email protected]>
Signed-off-by: Adrian Catangiu <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: divdeploy <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: hongkuang <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: gemini132 <[email protected]>
Co-authored-by: Matteo Muraca <[email protected]>
Co-authored-by: Liam Aharon <[email protected]>
Co-authored-by: Kian Paimani <[email protected]>
Co-authored-by: Alexandru Gheorghe <[email protected]>
Co-authored-by: Alessandro Siniscalchi <[email protected]>
Co-authored-by: Andrei Sandu <[email protected]>
Co-authored-by: Ross Bulat <[email protected]>
Co-authored-by: Serban Iorga <[email protected]>
Co-authored-by: s0me0ne-unkn0wn <[email protected]>
Co-authored-by: Sam Johnson <[email protected]>
Co-authored-by: Adrian Catangiu <[email protected]>
Co-authored-by: Javier Viola <[email protected]>
Co-authored-by: Alexandru Vasile <[email protected]>
Co-authored-by: Niklas Adolfsson <[email protected]>
Co-authored-by: Dastan <[email protected]>
Co-authored-by: Clara van Staden <[email protected]>
Co-authored-by: Ron <[email protected]>
Co-authored-by: Vincent Geddes <[email protected]>
Co-authored-by: Svyatoslav Nikolsky <[email protected]>
Co-authored-by: Michal Kucharczyk <[email protected]>
Co-authored-by: Dino Pačandi <[email protected]>
Co-authored-by: Andrei Eres <[email protected]>
Co-authored-by: Alin Dima <[email protected]>
Co-authored-by: Andrei Sandu <[email protected]>
Co-authored-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: Branislav Kontur <[email protected]>
Co-authored-by: Sebastian Kunert <[email protected]>
Co-authored-by: gupnik <[email protected]>
Co-authored-by: Vladimir Istyufeev <[email protected]>
Co-authored-by: Lulu <[email protected]>
Co-authored-by: Juan Girini <[email protected]>
Co-authored-by: Francisco Aguirre <[email protected]>
Co-authored-by: Dónal Murray <[email protected]>
Co-authored-by: Shawn Tabrizi <[email protected]>
Co-authored-by: Kutsal Kaan Bilgin <[email protected]>
Co-authored-by: Ermal Kaleci <[email protected]>
Co-authored-by: ordian <[email protected]>
Co-authored-by: divdeploy <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Sergej Sakac <[email protected]>
Co-authored-by: Squirrel <[email protected]>
Co-authored-by: HongKuang <[email protected]>
Co-authored-by: Tsvetomir Dimitrov <[email protected]>
Co-authored-by: Egor_P <[email protected]>
Co-authored-by: Aaro Altonen <[email protected]>
Co-authored-by: Dmitry Markin <[email protected]>
Co-authored-by: Alexandru Vasile <[email protected]>
Co-authored-by: Léa Narzis <[email protected]>
Co-authored-by: Gonçalo Pestana <[email protected]>
Co-authored-by: georgepisaltu <[email protected]>
Co-authored-by: command-bot <>
Co-authored-by: PG Herveou <[email protected]>
Co-authored-by: jimwfs <[email protected]>
Co-authored-by: jimwfs <[email protected]>
Co-authored-by: polka.dom <[email protected]>
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
…#4904)

Related: paritytech#4804.
Fixes the try state error in Westend:
https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6564522.
Passes here:
https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6580393

## Context
Currently in Kusama and Polkadot, an account can do both, directly
stake, and join a pool.

With the migration of pools to `DelegateStake` (See
paritytech#3905), the funds of pool
members are locked in a different way than for direct stakers.
- Pool member funds uses `holds`.
- `pallet-staking` uses deprecated locks (analogous to freeze) which can
overlap with holds.

An existing delegator can stake directly since pallet-staking only uses
free balance. But once an account becomes staker, we cannot allow them
to be delegator as this risks an account to use already staked (frozen)
funds in pools.

When an account gets into a situation where it is participating in both
pools and staking, it would no longer would be able to add any extra
bond to the pool but they can still withdraw funds.

## Changes
- Add test for the above scenario.
- Removes the assumption that a delegator cannot be a staker.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T1-FRAME This PR/Issue is related to core FRAME, the framework. T2-pallets This PR/Issue is related to a particular pallet.
Projects
Status: Done
Status: Audited
Development

Successfully merging this pull request may close these issues.

[NPoS] Nomination pools: Allow funds to be used for democracy