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.

Sign up for GitHub

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

Jump to bottom

[Pools] Remote Externalities test for DelegateStake strategy #4629

Closed
Ank4n opened this issue May 29, 2024 · 0 comments · Fixed by #4822
Closed

[Pools] Remote Externalities test for DelegateStake strategy #4629

Ank4n opened this issue May 29, 2024 · 0 comments · Fixed by #4822
Labels
C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. T2-pallets This PR/Issue is related to a particular pallet. T8-polkadot This PR/Issue is related to/affects the Polkadot network. T10-tests This PR/Issue is related to tests.

Comments

@Ank4n
Copy link
Contributor

Ank4n commented May 29, 2024 •
edited
Loading

Add remote-externalities test (can be added to pallet-nomination-pools-test-delegate-stake) that tests the following with the runtime state of polkadot and kusama. The pools::StakeAdapter in all cases should be DelegateStake (refer #3905). We can use westend-runtime but load state from kusama or polkadot for the following pallets: system, balances, staking, voterlist, nomination-pools.

  • Given the pool is unmigrated, no harmful extrinsic execution is possible that would put the pool in a corrupt (unrecoverable) state.
  • All pools can be migrated successfully with Calls::migrate_pool_to_delegate_stake.
  • Given the pool is migrated, but the pool member is not migrated, no harmful extrinsic execution is possible that would corrupt member's funds or state (or allow something that should normally be not possible).
  • All pool members can be migrated via Calls::migrate_delegation.
  • Scenario where a pool is slashed, and pending slashes are applied.
  • Any other scenarios?

Every test should run a <AllPalletsWithSystem as TryState<_>>::try_state() at the end.
@Ank4n Ank4n added the C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. label May 29, 2024
@Ank4n Ank4n mentioned this issue May 29, 2024
Closed
6 tasks
@Ank4n Ank4n added T2-pallets This PR/Issue is related to a particular pallet. T8-polkadot This PR/Issue is related to/affects the Polkadot network. T10-tests This PR/Issue is related to tests. labels May 29, 2024
@Ank4n Ank4n mentioned this issue Aug 14, 2024
Merged
4 tasks
github-merge-queue bot pushed a commit that referenced this issue Aug 14, 2024
@Ank4n @gpestana
[Pools] Fix issues with member migration to DelegateStake (#4822)
feacf2f 
## Context
Pool members using the old `TransferStake` strategy were able to
transfer all their funds to the pool. With `DelegateStake` changes, we
want to ensure similar behaviour by allowing members to delegate all
their stake to the pool.

## Changes
- Ensure all the balance including ED of an account can be delegated
(and used in the pool) by adding a provider for delegators.
- Gates calls that mutates the pool or pool member if they are in
unmigrated state. Closes
paritytech-secops/srlabs_findings#409.
- Adds remote test to migrate all pools and members to `DelegateStake`
which can be used with `Kusama` and `Polkadot` runtime state. closes
#4629.
- Add new runtime apis to read pool and member balance.

## Addressing possible migration errors 
Pool members migrating can run into two types of errors:
- Already Staking: If the pool member is already staking, we cannot
migrate them to `DelegateStake` since this may mean they are able to use
the same staked funds in the pool. Users would need to withdraw all
their funds from staking, in order to migrate their pool funds.
- Pool contribution below ED: For these cases transfer from pool account
to member account would fail. The affected users can top up their
accounts and redo migration.

Another error that was earlier possible was when member's free balance
is below ED. This PR adds a provider to delegator allowing all user
balance including ED can be contributed towards the pool. This helps
`1095` accounts in Polkadot and `41` accounts in Kusama to migrate now
which would have earlier failed.

## Results from RemoteExternalities Tests.

### Kusama
`Migration stats: success: 3017, direct_stakers: 361, unexpected_errors:
0`

### Polkadot
`Migration stats: success: 42859, direct_stakers: 643,
unexpected_errors: 0`

## TODO
- [x] Add runtime api for member total balance.
- [x] New
[issue](#5009) to reap
pool members with contribution below ED.
- [x] Add provider for delegators so whole balance including ED can be
held while contributing to pools.
- [x] Gate all pool extrinsics if pool/member is in non-migrated state.

---------

Co-authored-by: Gonçalo Pestana <[email protected]>
@github-project-automation github-project-automation bot moved this from 📕 Backlog to ✅ Done in (Nominated) Proof of Stake Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. T2-pallets This PR/Issue is related to a particular pallet. T8-polkadot This PR/Issue is related to/affects the Polkadot network. T10-tests This PR/Issue is related to tests.
Projects
(Nominated) Proof of Stake
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Pools] Fix issues with member migration to DelegateStake paritytech/polkadot-sdk
1 participant
@Ank4n