Stash/controller model for staking by gavofyork · Pull Request #1782 · paritytech/substrate

gavofyork · 2019-02-13T11:27:45Z

Implements #1709.

Also:

Moves Fee payment traits into SRML support, where they belong (they're not core).
Refactors EnsureAccountLiquid by adding a finer grained querier.
Refactors the spec generation code, getting it a bit more ready for when authorities use different crypto to accounts.

Associated spec/docs are at https://github.com/paritytech/substrate/blob/1a2ec9eec1fe9b3cc2677bac629fd7e9b0f6cf8e/srml/staking/Staking.md

TODO:

Refactor Intentions and Nominating over to Desired
Remove NominatorsFor, rename CurrentNominatorsFor to Stakers and populate at new era naively from Desired.
Avoid paying out per-session; pay out per-era to reduce trie churn.
Full documentation/specification.
Controller account can wield stash for governance stuff (democracy, council voting). Can probably be done easiest as a kind of sudo that checks to make sure the Call is to the governance module.
Tests - @shawntabrizi & @kianenigma
Introduce online Phragmén. Introduced in this branch
Introduce offline Phragmén, along with desire/unlock freeze for period at end of era.

xlc · 2019-02-15T22:49:40Z

srml/balances/src/lib.rs


-	fn total_issuance() -> Self:: Balance {
+	fn total_issuance() -> Self::Balance {
 		Self::total_issuance()


isn't this an infinite recursive call? or is there any mechanism to ensure this calls total_issuance generated by decl_storage?

That's not new, but it does look dodgy...

…rate into gav-new-staking

…o gav-new-staking

kianenigma

I fixed the global tests and merge conflicts in the last commit and of course had to build the wasm files again. I only learned afterward that once wasm is re-built a impl version bump is usually needed. Is this the case when now the PR impl_version is already bumped w.r.t. master?
In either case, if someone with a bit more experience with versioning confirms that this spec/impl versioning is ok then this is all good to merge. Rest of the minor test cases can be created via the separate phragmen PR.

https://github.com/paritytech/substrate/pull/1782/files#diff-5e5e1c3aec9ddfde0a9054d062ab3db9R63

shawntabrizi

Looks good to merge in with a temporary validator choosing algorithm.
There should be a second pass at tests once Phragmen is in, and there are TODO items everywhere this will need to be addressed.

Additionally, I think it makes sense to drop ensure_account_liquid with ensure_account_can_withdraw, but that can be discussed elsewhere.

Almost forgot this one

gavofyork · 2019-03-01T08:38:16Z

srml/staking/src/lib.rs

 			let controller = ensure_signed(origin)?;
 			let _ledger = Self::ledger(&controller).ok_or("not a controller")?;
 			<Nominators<T>>::remove(&controller);
+			ensure!(prefs.unstake_threshold <= MAX_UNSTAKE_THRESHOLD, "unstake threshold too large");


ensure! after you've made changes to storage. Rookie error :-P

gavofyork · 2019-03-01T08:38:53Z

srml/staking/src/lib.rs

 }

 impl<T: Trait> EnsureAccountLiquid<T::AccountId, BalanceOf<T>> for Module<T> {
+	// TODO: Consider replacing uses of this function in favor for ensure_account_can_withdraw


It remains here on purpose. Contracts module cannot be changed in order to use the finer-grained function.

If it remains on purpose, we should create an issue for it :)

kianenigma · 2019-03-01T09:40:07Z

srml/staking/src/tests.rs

+
+/*
+#[test]
+fn slot_stake_is_least_staked_validator_and_limits_maximum_punishment() {


@shawntabrizi shouldn't we uncomment this now?

Yeah, please commit no commented-out-code.

…r 011648b

…o gav-new-staking

burdges · 2019-03-11T16:06:12Z

I'd think withdraw_unbonded would be functions controlled by the stash account's key? Or maybe the name just confused me and it leaves the funds under the control of the stash account, but now unbonded?

* First steps to stash/controller separation * More drafting * More drafting * Finish draft. * Optimisation * Remove accidental commit * Make it build. * Fix linked map for traits. * Fix Option<_> variant. * Improve naming a tad * Rebuild runtime * Builds! * First test. * Bump RT version * Minor fix * Update Mock * adds the correct reward testcase (+staking eras which was already ok) * fixes the basic staking testcase to work properly (along with a small fix in the module) * New logic to avoid controller transferring stash. * Fix some build issues. * adding some comments to tests * Fix impls. * adds a few more lines to explain the test case * More fixes. * gets the basic test up and running again * Fix rest of build * Rebuild wasm * Fix docs. * fix staking test with new chnages * updating some tests, pending questions * More working tests * adds double staking test * Docs * remove invalid slashing test * Payee stuff. * Fix build * Docs * Fix test * Fix a couple of tests * Layout plan for finishing tests before Pragmen * Add some working tests * re-build staking and reward tests * Add more tests * fix offline grace test * Nominator should have payee checked for cleanup * adds more nomination tets * adds validator prefs tests * Fix and clean up some TODOs * Fix a couple of issues * Fix tests * noting warnings from tests * final fix of local tests * Fix slot_stake bug * Half baked test * Add logic to limit `unstake_threshold` set in storage * Make sure to check before writing! Almost forgot this one * Move a couple of comments * fix last broken slot_stake test * Ignore broken test

gavofyork added 2 commits February 7, 2019 17:32

First steps to stash/controller separation

e89dff9

Merge remote-tracking branch 'origin/master' into gav-new-staking

8e9c2d5

gavofyork added the A3-in_progress Pull request is in progress. No review needed at this stage. label Feb 13, 2019

devops-parity added the B2-breaksapi label Feb 13, 2019

gavofyork added 5 commits February 14, 2019 11:07

More drafting

29d1b20

More drafting

ac5d897

Finish draft.

bfc089f

Optimisation

fe19f73

Remove accidental commit

24cedfb

gavofyork closed this Feb 15, 2019

gavofyork reopened this Feb 15, 2019

gavofyork closed this Feb 15, 2019

gavofyork reopened this Feb 15, 2019

gavofyork and others added 10 commits February 15, 2019 14:46

Make it build.

0876c86

Fix linked map for traits.

c9f643f

Fix Option<_> variant.

9190f7d

Merge branch 'master' into td-fixlinked

4b5274e

Improve naming a tad

5354a2e

Rebuild runtime

00ba07e

Merge remote-tracking branch 'origin/master' into gav-new-staking2

78bcf1a

Merge remote-tracking branch 'origin/td-fixlinked' into gav-new-staking2

5ec3741

Builds!

ca18c79

First test.

a24dd8a

devops-parity added the A4-gotissues label Feb 15, 2019

gavofyork added 2 commits February 15, 2019 18:48

Bump RT version

15a3100

Minor fix

bd0a3de

xlc reviewed Feb 15, 2019

View reviewed changes

kianenigma mentioned this pull request Feb 18, 2019

Optimal Approved Assignment using Phragmén #1772

Closed

shawntabrizi and others added 2 commits February 19, 2019 16:50

Update Mock

68f5405

adds the correct reward testcase (+staking eras which was already ok)

001a665

shawntabrizi and others added 5 commits February 28, 2019 16:43

Merge branch 'gav-new-staking' of https://github.com/paritytech/subst…

a53c3fd

…rate into gav-new-staking

Fix slot_stake bug

93da8d8

Half baked test

4603cfe

fix a few gobal tests

8bb2e43

Merge branch 'gav-new-staking' of github.com:paritytech/substrate int…

2f43287

…o gav-new-staking

gavofyork added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. A4-gotissues labels Feb 28, 2019

gavofyork requested review from kianenigma and shawntabrizi February 28, 2019 19:17

kianenigma suggested changes Feb 28, 2019

View reviewed changes

Add logic to limit unstake_threshold set in storage

a4ca045

shawntabrizi approved these changes Feb 28, 2019

View reviewed changes

Make sure to check before writing!

ee54da0

Almost forgot this one

gavofyork commented Mar 1, 2019

View reviewed changes

kianenigma reviewed Mar 1, 2019

View reviewed changes

gavofyork and others added 2 commits March 1, 2019 10:52

Move a couple of comments

51d25ed

devops-parity updated wasm runtime blobs b58d9517 and merged in maste…

231a02d

…r 011648b

kianenigma approved these changes Mar 1, 2019

View reviewed changes

kianenigma and others added 4 commits March 1, 2019 11:32

fix last broken slot_stake test

68b5938

Merge branch 'gav-new-staking' of github.com:paritytech/substrate int…

3a74b73

…o gav-new-staking

Merge remote-tracking branch 'origin/master' into gav-new-staking

ae1e1e5

Ignore broken test

da9cb52

gavofyork merged commit 701fd11 into master Mar 2, 2019

gavofyork deleted the gav-new-staking branch March 2, 2019 13:32

kianenigma mentioned this pull request Mar 4, 2019

Phragmén Validator Election #1915

Merged

5 tasks

jacogr mentioned this pull request Mar 7, 2019

Adjust staking with new staking interfaces polkadot-js/apps#715

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stash/controller model for staking#1782

Stash/controller model for staking#1782
gavofyork merged 85 commits intomasterfrom
gav-new-staking

gavofyork commented Feb 13, 2019 •

edited by kianenigma

Loading

Uh oh!

xlc Feb 15, 2019

Uh oh!

gavofyork Feb 18, 2019

Uh oh!

kianenigma left a comment •

edited

Loading

Uh oh!

shawntabrizi left a comment

Uh oh!

gavofyork Mar 1, 2019

Uh oh!

gavofyork Mar 1, 2019

Uh oh!

bkchr Mar 1, 2019

Uh oh!

kianenigma Mar 1, 2019

Uh oh!

bkchr Mar 1, 2019

Uh oh!

kianenigma Mar 1, 2019

Uh oh!

burdges commented Mar 11, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Conversation

gavofyork commented Feb 13, 2019 • edited by kianenigma Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kianenigma left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shawntabrizi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

burdges commented Mar 11, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

gavofyork commented Feb 13, 2019 •

edited by kianenigma

Loading

kianenigma left a comment •

edited

Loading