paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.com/
1.89k stars 690 forks source link

[NPoS] Deprecate Stash/Controller #471

Open kianenigma opened 4 years ago

kianenigma commented 4 years ago

Given the recent proxy features, an account can easily allow another account to execute only certain calls on its behalf. Given this, stash/controller is not really needed anymore.

This is a low priority improvement at the time of this writing. One scenario where it could become important is if we need to optimise staking. Currently, we frequently need to first query the Bonded storage to get the controller/stash mapping. Using proxies, we won't need to do this anymore.

burdges commented 4 years ago

I have not tracked the recent changes but yes we should simplify the code when possible. We do however want solid key management to be explained well, be user-friendly, and be encuraged.

We might retain the stash/controller/proxy/etc. terminology in the documentation and maybe even user interfaces, perhap even just for consistency. We might conversely reconsider this terminology too, well maybe terms like nomination proxy, control proxy, governance proxy, etc. all make more sense. I think parity has a contractor who does cryptography UX btw, so maybe ask her.

DemiMarie commented 4 years ago

I consider “nomination proxy” and “validator control proxy” to be more intuitive than “nominator controller” and “validator controller”. That said, proxied transactions currently involve an extra storage read, so we might not gain anything performance-wise. Furthermore, not using a proxy is cheaper, so this discourages good key management.

bkchr commented 4 years ago

not using a proxy is cheaper, so this discourages good key management.

If you are using the insecure variant because of some small fee differences and you loose your account because of that, there is nothing really where we can help.

DemiMarie commented 4 years ago

If we made nominator and validator transactions require a proxy, I would support this.

jam10o-new commented 4 years ago

not using a proxy is cheaper, so this discourages good key management.

If you are using the insecure variant because of some small fee differences and you loose your account because of that, there is nothing really where we can help.

Proxies have deposits where controllers do not require a deposit, which I think is more significant than the fee differences. It also means there's a barrier to entry to actually using a proxy where there wasn't in using a controller, unless we create staking proxies internally in the runtime waiving the usual deposit, or something.

DemiMarie commented 4 years ago

not using a proxy is cheaper, so this discourages good key management.

If you are using the insecure variant because of some small fee differences and you loose your account because of that, there is nothing really where we can help.

Proxies have deposits where controllers do not require a deposit, which I think is more significant than the fee differences. It also means there's a barrier to entry to actually using a proxy where there wasn't in using a controller, unless we create staking proxies internally in the runtime waiving the usual deposit, or something.

I would be fine with reimplementing controller accounts in terms of the more general proxy mechanism, but only if using a controller account still remained the path of least resistance.

burdges commented 3 years ago

It's less "deprecate" than "unify" I think. Afaik, there are basically negligible differences between the controller and stacking proxy, so they could be merged once someone figures out how to do that cleanly. In particular, we'd want old parity signers to still support controller key operations, although that'd likely just happen anyways.

There is a minor concern that stacking proxies permit rebonding which could result in increased slashes. We need to figure out what the right story there is. If we're being pedanting, then maybe we remove that functionality form the staking proxy when merging, but maybe it should be retained. This requires discussion.

kianenigma commented 2 years ago

If and when we break down staking into smaller pallets (private writeup about it: https://forum.parity.io/c/frame/staking/51), this will be much simpler to approach.

kianenigma commented 2 years ago

The main problem currently is that a normal proxy requires a hefty deposit, while a staking controller is basically a free proxy.

I suggest the following steps to solve this, roughly:

trait ProxyProvider {
  type AccountId;

  // get the proxy of stash, if any. 
  fn proxy_for(stash: &Self::AccountId) -> Option<Self::AccountId>;
  fn create_proxy(...) -> Result<_, _>;
  fn remove_proxy(...) -> Result<_, _>;
}

As a first step, staking's inner controller management system will be the ProxyProvider

impl ProxyProvider for pallet_staking::Pallet<_> { ... }

Then, we should be able to throw away most of the controller logic and replace it with an external ProxyProvider that's implemented by the pallet-proxy.

This basically means by becoming a staker, you get a free proxy. To make it economically sensible, your bond must be more than the required deposit of a proxy. Optionally, you can get this proxy only if your bond is more than that. Otherwise, you will lose the proxy.

Polkadot-Forum commented 2 years ago

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

https://forum.polkadot.network/t/staking-deprecating-controller-for-proxies-strategy-implementation/724/1

joepetrowski commented 2 years ago

This basically means by becoming a staker, you get a free proxy.

How do you plan to handle this on the proxy pallet side? For example, if you add an additional proxy now, it calculates the total deposit required. So even though they got the staking proxy for free, if they try to add another proxy to the same account, it will try to take the 20 DOT deposit, and this could be quite frustrating for UX.

Perhaps staking UIs should give the user the chance to choose their proxy type of which Staking is a subset at the time of bonding? E.g. this would make a NonTransfer proxy type free so the user could also vote in referenda with their stash.

Overall though I think this makes the staking experience better, but if we expect the stash to be really cold and don't handle other cases then there is a risk of making other experiences (like setting a governance delegation) worse.

burdges commented 11 months ago

Any idea how migration works? We should migrate all current controllers to being staking proxies presumably.

I suppose the deposits would not match perfectly, but we've made enough mistakes in that sort of thing that someone should just take the extra time to make sure they work even with the grandfathered-in small deposit.

rossbulat commented 11 months ago

Any idea how migration works? We should migrate all current controllers to being staking proxies presumably.

I suppose the deposits would not match perfectly, but we've made enough mistakes in that sort of thing that someone should just take the extra time to make sure they work even with the grandfathered-in small deposit.

The plan originally (see https://forum.polkadot.network/t/staking-controller-deprecation-plan-staking-ui-leads-comms) was to draw down the amount of unique pairs via UI prompts, and after an adequate period of time submit an OpenGov referendum to move the remaining unresponsive controller accounts to their stash. It is about time I revisited the numbers and best way forward but this strategy did not involve adding proxy accounts at all, as to limit the storage impacts of the deprecation (we don't want a bunch of unused proxies just sitting on chain with no incentive to remove them).

For whatever reason, roadmaps, not in their interests, no incentives etc, not many UIs have displayed a prompt and action to switch the controller back to the stash - wallet extensions mostly focus on pools, JS apps has not, etc. So the effectiveness of the UI prompt in staking dashboard, while it has moved the needle and has communicated the deprecation messaging, has not had as much of an impact as it could have ecosystem-wide.

Nonetheless, think it's worth revisiting the numbers, it has been around 6 months since set_controller and bond stopped accepting a different controller to the stash. If the % of unique pairs were low enough I'd probably still be in favour of the original approach of not involving proxies, but then again a low % wouldn't have a huge impact on proxy storage anyway.

joepetrowski commented 11 months ago

If the % of unique pairs were low enough I'd probably still be in favour of the original approach of not involving proxies, but then again a low % wouldn't have a huge impact on proxy storage anyway.

Same, it will also be simpler when it comes to migrating staking to a parachain. Of course we'll have to deal with existing Staking proxies, but this will add another class, "deposit-free staking proxies", to migrate.

kianenigma commented 9 months ago

I wonder if the move to fungibles would make this easier: the proxy pallet could have a shared hold reason with staking, allowing them to both overlap and use one another?

There should also be a MinimumBond that is at least around the reasonable deposit for proxies (20 DOTs).

rossbulat commented 9 months ago

I wonder if the move to fungibles would make this easier: the proxy pallet could have a shared hold reason with staking, allowing them to both overlap and use one another?

To clarify this use case, would the proxy delegate be eligible to sign Staking proxy calls directly without needing to use proxy.proxy?

It has been a while since this thread was updated - we are currently waiting for https://github.com/paritytech/polkadot-sdk/pull/1196 and https://github.com/paritytech/polkadot-sdk/pull/2589 to make it into Kusama and Polkadot runtimes. The latter has recently been added to Westend. Once deployed globally, it will be possible to go ahead and deprecate all unique controller pairs. I believe we may be awaiting auditing before they are deployed beyond Westend.