Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

fast-unstake: decouple from nomination pools + staking #12337

Closed
1 of 2 tasks
kianenigma opened this issue Sep 23, 2022 · 4 comments · Fixed by #12424
Closed
1 of 2 tasks

fast-unstake: decouple from nomination pools + staking #12337

kianenigma opened this issue Sep 23, 2022 · 4 comments · Fixed by #12424
Assignees
Labels
I7-refactor Code needs refactoring. J0-enhancement An additional feature request.

Comments

@kianenigma
Copy link
Contributor

kianenigma commented Sep 23, 2022

The first draft of this pallet was written with the tight coupling approach for the sake of simplicity.

  • Moving Forward
  1. We can use StakingInterface as a proxy into a staking pallet.
  2. We won't rely on nomination pools anymore and won't provide the feature to join the pool on the fly.

If we happen to want to still support pool join, we can either:

  1. You provide some opaque: Vec<u8> data to the fast-unstake pallet, which is later on forwarded through some hook to nomination pools, and decoded as pool_id. Still, very very janky.
  2. we wrap the current fast-unstake in a new pallet that does fast-unstake-and-join-pools.
@kianenigma kianenigma added I7-refactor Code needs refactoring. J0-enhancement An additional feature request. labels Sep 23, 2022
@kianenigma kianenigma moved this to 📕 Backlog in (Nominated) Proof of Stake Sep 23, 2022
@Szegoo
Copy link
Contributor

Szegoo commented Sep 23, 2022

I am going to attempt this.

@kianenigma
Copy link
Contributor Author

@Szegoo not recommended, as we're doing a lot of development on this and it might change. I'd recommend sticking to mentor tagged issues.

@ruseinov
Copy link
Contributor

ruseinov commented Oct 4, 2022

  1. We can use StakingInterface as a proxy into a staking pallet.

With the way fast-unstake is currently using pallet-staking Storage/Constants/Methods it seems like StakingInterface is not going to be enough to decouple one from the other.

What we would need to do for full decoupling:

  1. Remove direct calls to pallet-staking, like: Staking::<T>::chill, Staking::<T>::slashing_spans. For that we'd need to introduce some new methods to the StakingInterface
  2. Remove param/constant getters that reference pallet-staking, like: <T as pallet_staking::Config>::BondingDuration::get()
  3. Remove storage calls: pallet_staking::Ledger::<T>, pallet_staking::ValidatorCount, pallet_staking::ErasStakers

One of the options is incorporating all the storage/constant calls as methods in the StakingInterface.
And then we could also call it type Staking: StakingInterface<>,

@kianenigma
Copy link
Contributor Author

All of the above can reasonably be added to StakingInterface. It is reasonable for a interface to staking to be able to chill or force_unstake stakers, and to report what is its current bodning duration and validator count.

The slashing span thing can be abstracted away and should be dealt with purely on the staking side. We need that value only for weighing anyhow.

I am not saying we should YOLO it and tweak StakingInterface arbitrarily, but all of the above make sense to me to live in the interface, at least at the time being. If you think otherwise, happy to discuss, else I think we can move forward.

@ruseinov ruseinov moved this from 📕 Backlog to ✂️ In progress. in (Nominated) Proof of Stake Oct 5, 2022
Repository owner moved this from ✂️ In progress. to ✅ Done in (Nominated) Proof of Stake Oct 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
I7-refactor Code needs refactoring. J0-enhancement An additional feature request.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants