Staking::{bond, set_controller} to set controllers to stash only. #14039

rossbulat · 2023-04-28T09:01:03Z

This PR updates the staking pallet's set_controller and bond calls to not take the controller argument. set_controller and bond always set the controller to the stash account and signer respectively.

Doing this turns off the taps to new unique stash & controller pairs, aiding in the controller account deprecation and migration to proxy accounts.

rossbulat · 2023-04-28T09:22:00Z

bot bench $ pallet dev staking

command-bot · 2023-04-28T09:22:05Z

@rossbulat https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2748302 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev staking. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 4-6416a908-7000-4c74-93ea-efb4df2c951c to cancel this command or bot cancel to cancel all commands in this pull request.

command-bot · 2023-04-28T09:47:28Z

@rossbulat Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev staking has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2748302 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2748302/artifacts/download.

kianenigma · 2023-04-28T12:32:08Z

putting the hat of the devil here, how would you evaluate this against just not supporting it in the UI? this would probably be a breaking change for Ledger, Nova, PJS Apps and so on.

The pro is that it is strong bold message to all of these entities about what is to come.
But con is that it is arguably unnecessarily disruptive.

Would be good to get opinion from other Staking UI stakeholders as well. perhaps this can all be coordinated offchain.

As a side technical note, if you are to do this, you should also remove the controller argument upon fn bond to truly put a cap on new unique controllers.

Ank4n · 2023-04-29T11:01:11Z

putting the hat of the devil here, how would you evaluate this against just not supporting it in the UI?

I would also prefer the proxy solution being supported first on UI before deprecating it on runtime level.

this would probably be a breaking change for Ledger, Nova, PJS Apps and so on.

As a first step of deprecation, may be we can require deposit for setting controller (non-stash account) that is larger than proxy deposit to encourage other Staking UI/wallets to start supporting proxies but avoid being a breaking change for them. If governance allows, we could then retroactively apply deposit for existing users with controllers as well incentivising them to migrate to proxy instead.

rossbulat · 2023-04-29T11:33:04Z

The proxy solution is already supported on staking dashboard. It will be deployed next week.

I am of the view that a well-coordinated update process with the staking UIs is the best way forward here. This PR (with the bond call also updated) will be the only one needed to deprecate controller accounts. Once people have moved over to their stash, the only tasks will be internal to the pallet, tidying up storage, removing controller logic, etc.

Adding more steps such as deposits and what-not will ultimately not avoid the eventual outcome of adjusting these calls. And the larger the staking system grows, the harder it will be to move the controllers - the sooner we do this, the better.

I will be mentioning this PR soon in a forum post with all my notes & steps forward, to rally all the staking UIs and ensure that they are aware of the updates.

Worst case scenario is that UIs will not update, users begin reporting that the app is not working, so they will then push a fix in quick-time. So I am confident market factors will work themselves out. Saying this, large stakeholders, Ledger, Talisman, should be very aware of this leading up to a runtime upgrade, and we should do our best to communicate any planned changes to them prior to the upgrade happening.

Polkadot-Forum · 2023-04-29T12:57:00Z

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

https://forum.polkadot.network/t/staking-controller-deprecation-plan-staking-ui-leads-comms/2748/1

shawntabrizi · 2023-04-29T20:43:46Z

I think a better option here, for less breaking changes, is to keep the function signature for set controller the same, but have it always return an error. Basically, removing the call's functionality. This avoids both a change to the function signature, which is breaking for downstream apps, and also eliminates a subtle change in behavior which could look like a false positive.

If you instead make the function signature the same and it just returns an error, all existing UIs should at least be able to handle that appropriately.

If you want a function to set controller to self, a new one can be introduced, as an off-boarding ramp for the new proxy behavior.

rossbulat · 2023-04-30T02:41:29Z

I think a better option here, for less breaking changes, is to keep the function signature for set controller the same, but have it always return an error. Basically, removing the call's functionality. This avoids both a change to the function signature, which is breaking for downstream apps, and also eliminates a subtle change in behavior which could look like a false positive.

If you instead make the function signature the same and it just returns an error, all existing UIs should at least be able to handle that appropriately.

If you want a function to set controller to self, a new one can be introduced, as an off-boarding ramp for the new proxy behavior.

We could return an error, for set_controller and bond, if the controller provided is not the caller. Then UIs can handle a non-ambiguous error of ControllersDeprecated while maintaining the function signatures.

The issue though, for bond also, is that we will eventually need to remove the controller argument anyway. I feel this will be kicking the can down the road a little bit - by which time the staking system will be much larger & UIs many more users.

I think the scenario of the api call erroring out (different signature) also would not be the worst thing - there would be an error provided by Polkadot API saying caught Error: Extrinsic staking.setController expects 0 arguments, got 1.. It would be very obvious to UIs where the breakage is, perhaps even more than the enum error.

shawntabrizi · 2023-04-30T15:18:16Z

I agree that basically all approaches are about equal, and at some point there will be something breaking. So really up to how you want to coordinate with community.

frame/staking/README.md

valentunn · 2023-05-02T11:13:16Z

Hey everyone! Nova team here. We want to point out that rolling out an update that fixes breaking changes on mobile is not a fast process since users are not forced to update to newer versions. Given that, as well as how critical this breaking change is, we would highly appreciate it if this change would be released on Kusama/Polkadot at least one month after its release on Westend, where we will be able to perform end-to-end testing (a few days) and then roll out the update (takes 2-3 weeks for majority of userbase to update)

rossbulat · 2023-05-03T03:40:25Z

Hey everyone! Nova team here. We want to point out that rolling out an update that fixes breaking changes on mobile is not a fast process since users are not forced to update to newer versions. Given that, as well as how critical this breaking change is, we would highly appreciate it if this change would be released on Kusama/Polkadot at least one month after its release on Westend, where we will be able to perform end-to-end testing (a few days) and then roll out the update (takes 2-3 weeks for majority of userbase to update)

Noted. Once we know the runtime version you could do a conditional to ensure no breakages:

const tx = runtime <= prevRuntime
   ? api.tx.staking.setController(controller)
   : api.tx.staking.setController();

// and same with `bond`.

rossbulat · 2023-05-11T13:08:56Z

bot rebase

paritytech-processbot · 2023-05-11T13:09:01Z

Branch is already up-to-date

rossbulat · 2023-05-11T18:22:08Z

bot merge

Polkadot-Forum · 2023-05-12T05:01:09Z

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

https://forum.polkadot.network/t/staking-controller-deprecation-plan-staking-ui-leads-comms/2748/12

rossbulat · 2023-06-08T08:24:39Z

Westend now reflects the changes of `bond` and `set_controller`.

set_controller(controller) has become set_controller(). This call simply sets the controller account to the caller’s stash account, if it is not already.
bond(controller, value, payee) has become bond(value, payee). This call now sets the caller as the controller account (same as the stash).

UIs can now test any necessary changes on Westend leading up to the mainnet releases.

We can assume that the next Polkadot and Kusama releases will reflect these changes, and that it will be at least 4-6 weeks before they are deployed on the networks.

…ritytech#14039) * update set_controller * clone * bond uses `stash` * remove controller from bond(), chill_other test works * remove ctlr from testing_utils & dead ctlr -> dead payee * mvs controllers to stashes for 3 tests * migrate mock bond fns & fix 1 test * mvs controllers to stashes for 7 tests * mvs controllers to stashes for 9 tests * remove double_controlling_should_fail * remove double_staking_should_fail * mvs controllers to stashes for 10 tests * mvs controllers to stashes for 2 tests * remove payout_creates_controller * mvs controllers to stashes for 27 tests * remove println! * fix rewards_should_work * fix test_payout_stakers * fix bond benchmark * clone * rm unused import * rm unused var * rm controller from create_offender * fix GenesisConfig stakers * fix controllers in consensus pallets * fix unqiue controller in chain_spec * fmt * fix create_offender * fix set_controller benchmark * add TODO * create_unique_stash_controller * staking benchmarks working * fmt * fix args * rm println * import * import * fix fast unstake tests * fix staking-tests-e2e * fix root-offenses * fmt * differentiate controller to stash * bring back change_controller_works w. unique ctrl * bring back double_staking_should_fail * double_controlling_attempt_should_fail * bring back payout_creates_controller * add commnet to controller balances * + set_controller call description * fmt * rm clones * fmt * clippy fixes * fmt * update README * small fixes * use controller_to_be_deprecated * .comment * comment * bump zombienet version * ci --------- Co-authored-by: parity-processbot <> Co-authored-by: Javier Viola <javier@parity.io>

update set_controller

0811cb9

rossbulat requested a review from a team April 28, 2023 09:01

paritytech deleted a comment from command-bot bot Apr 28, 2023

clone

6f0be00

rossbulat requested review from gpestana, kianenigma and Ank4n April 28, 2023 09:25

paritytech deleted a comment from command-bot bot Apr 28, 2023

paritytech deleted a comment from paritytech-cicd-pr Apr 28, 2023

paritytech deleted a comment from command-bot bot Apr 28, 2023

rossbulat changed the title ~~Staking::set_controller to update controllers to stash only.~~ Staking::{bond, set_controller} to update controllers to stash only. Apr 29, 2023

gpestana reviewed Apr 30, 2023

View reviewed changes

frame/staking/README.md Outdated Show resolved Hide resolved

ci

fa530a3

paritytech-processbot bot merged commit 8796276 into master May 11, 2023

paritytech-processbot bot deleted the rb-set-controller-to-stash branch May 11, 2023 18:22

fgamundi mentioned this pull request Jun 14, 2023

Increase staking pallet version after #14039 #14385

Closed

2 tasks

github-actions bot mentioned this pull request Jun 15, 2023

Update substrate/polkadot/cumulus from v0.9.40 to v0.9.43 moonbeam-foundation/moonbeam#2354

Closed

kiltbot mentioned this pull request Jun 21, 2023

1️⃣ [AUTOMATIC] Update Polkadot dependencies from 0.9.42 to 0.9.43 KILTprotocol/kilt-node#531

Closed

kacperzuk-neti mentioned this pull request Jun 23, 2023

Polkadot v0.9.43 liberland/liberland_substrate#295

Merged

15 tasks

Imod7 mentioned this pull request Jul 1, 2023

fix!: removes controller arg from bond and setController methods paritytech/txwrapper-core#309

Merged

fgamundi mentioned this pull request Jul 3, 2023

Update RelayEncoder to be 9430 compatible moonbeam-foundation/moonbeam#2376

Merged

Agusrodri mentioned this pull request Jul 4, 2023

Dependency (Substrate/Polkadot/Frontier/Cumulus/...) update to v0.9.43 moonbeam-foundation/moonbeam#2377

Merged

1 task

lexnv mentioned this pull request Jul 6, 2023

Utilize Metadata V15 paritytech/subxt#1041

Merged

This was referenced Aug 14, 2023

fix(sdk-coin-dot): make controller optional for bond call BitGo/BitGoJS#3803

Merged

update DOT sdk BitGo/BitGoJS#3816

Merged

chabroA mentioned this pull request Nov 15, 2023

[FEATURE] Polkadot network upgrade: sunset of stash/controller feature LedgerHQ/ledger-live#5462

Merged

9 tasks

rossbulat mentioned this pull request Nov 27, 2023

Staking: Controller Deprecation Step by Step & Tracking Issue paritytech/polkadot-sdk#2500

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Staking::{bond, set_controller} to set controllers to stash only. #14039

Staking::{bond, set_controller} to set controllers to stash only. #14039

rossbulat commented Apr 28, 2023 •

edited

Loading

rossbulat commented Apr 28, 2023

command-bot bot commented Apr 28, 2023 •

edited

Loading

command-bot bot commented Apr 28, 2023

kianenigma commented Apr 28, 2023

Ank4n commented Apr 29, 2023

rossbulat commented Apr 29, 2023 •

edited

Loading

Polkadot-Forum commented Apr 29, 2023

shawntabrizi commented Apr 29, 2023 •

edited

Loading

rossbulat commented Apr 30, 2023 •

edited

Loading

shawntabrizi commented Apr 30, 2023

valentunn commented May 2, 2023 •

edited

Loading

rossbulat commented May 3, 2023 •

edited

Loading

rossbulat commented May 11, 2023

paritytech-processbot bot commented May 11, 2023

rossbulat commented May 11, 2023

Polkadot-Forum commented May 12, 2023

rossbulat commented Jun 8, 2023

Staking::{bond, set_controller} to set controllers to stash only. #14039

Staking::{bond, set_controller} to set controllers to stash only. #14039

Conversation

rossbulat commented Apr 28, 2023 • edited Loading

rossbulat commented Apr 28, 2023

command-bot bot commented Apr 28, 2023 • edited Loading

command-bot bot commented Apr 28, 2023

kianenigma commented Apr 28, 2023

Ank4n commented Apr 29, 2023

rossbulat commented Apr 29, 2023 • edited Loading

Polkadot-Forum commented Apr 29, 2023

shawntabrizi commented Apr 29, 2023 • edited Loading

rossbulat commented Apr 30, 2023 • edited Loading

shawntabrizi commented Apr 30, 2023

valentunn commented May 2, 2023 • edited Loading

rossbulat commented May 3, 2023 • edited Loading

rossbulat commented May 11, 2023

paritytech-processbot bot commented May 11, 2023

rossbulat commented May 11, 2023

Polkadot-Forum commented May 12, 2023

rossbulat commented Jun 8, 2023

Westend now reflects the changes of bond and set_controller.

rossbulat commented Apr 28, 2023 •

edited

Loading

command-bot bot commented Apr 28, 2023 •

edited

Loading

rossbulat commented Apr 29, 2023 •

edited

Loading

shawntabrizi commented Apr 29, 2023 •

edited

Loading

rossbulat commented Apr 30, 2023 •

edited

Loading

valentunn commented May 2, 2023 •

edited

Loading

rossbulat commented May 3, 2023 •

edited

Loading

Westend now reflects the changes of `bond` and `set_controller`.