-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Documentation for Staking Module #1951
Conversation
This comment has been minimized.
This comment has been minimized.
1 similar comment
|
It looks like @kianenigma signed our Contributor License Agreement. 👍 Many thanks, Parity Technologies CLA Bot |
bkchr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a really good documentation! :)
srml/staking/README.adoc
Outdated
| - Once a new era is triggered, rewards are paid to the validators and the associated nominators. | ||
| - The validator can declare an amount that does not get shared with the nominators at each reward payout through their `ValidatorPrefs`. This value gets deducted from the total reward that can be paid. The remaining portion is split among the validator and all of the nominators who had a vote for this validator, proportional to their staked value. | ||
| - All entities who receive a reward have the option to choose their reward destination, through the `Payee` storage, to be one of the following: | ||
| - Controller account. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not displayed as sub-listing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure? looks good to me :-?
srml/staking/README.adoc
Outdated
|
|
||
| ### Election algorithm details. | ||
|
|
||
| Current election algorithm is implemented based on Phragmen. The reference implementation can be found [here](https://github.com/w3f/consensus/tree/master/NPoS). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather link to a more formal description of the algorithm? (if there is any).
It's like, we have this implementation in rust, but if you don't understand rust, just read the implementation in language xy (in this case python) to better understand what is happening :D
The problem is, that a specific language is often not that well suited for explaining high level algorithms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
afaik the only resource is this single python script. I have a yet open PR on that repo that adds a lot of comments to the code to make it more readable.
w3f/consensus#7
srml/staking/README.adoc
Outdated
|
|
||
| Any account pair successfully placed at stake can accept three possible roles, namely: `validate`, `nominate` or simply `chill`. Note that during the process of accepting these roles, the _controller_ account is always responsible for declaring interest and the _stash_ account stays untouched, without directly interacting in any operation. | ||
|
|
||
| A **validator** takes the role of either validating blocks or ensuring their finality, maintaining the veracity of the network in other words. A validator should avoid both any sort of malicious misbehavior and going offline. Unlike nominating, bonded accounts that state interest in being a validator do NOT get immediately chosen as a validator. Instead, they are declared as a _candidate_ and they _might_ get elected at the _next **era**_ as a validator. The result of election is determined by nominators and their votes. An account can become a validator via the `validate()` call. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not really true; nominators aren't guaranteed to nominate any more than validators are guaranteed to validate. it could be that a nominator only nominates validators that are not chosen. it could also be that the optimisation algorithm doesn't select a nominator's stake to back a validator, even if the validator does become active.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unlike nominating, bonded accounts that state interest in being a validator do NOT get immediately chosen as a validator ...
Agreed. This is wrong + creates confusion (referring to specifically the bold part.)
2b9cb6d to
28b959b
Compare
|
This now also complies with the final template of the SRML docs (to a high extent). |
| //! | ||
| //! # References | ||
| //! | ||
| //! 1. This document is written as a more verbose version of the original [Staking.md](../Staking.md) file. Some sections, are taken directly from the aforementioned document. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't you remove this file in the last pr?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh, the intention was to maintain that as well. Though Gavin said he is also ok with the old one not being kept if everything is covered in a new doc.
I will still keep this section and see where the missing file is + if it is recoverable.
joepetrowski
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some typos and sentence structure stuff to fix
srml/staking/src/lib.rs
Outdated
| //! # Staking Module | ||
| //! | ||
| //! <!-- Original author of paragraph: @gavofyork --> | ||
| //! The staking module is the means by which a set of network maintainers (known as "authorities" in some contexts and "validators" in others) are chosen based upon those who voluntarily place funds under deposit. Under deposit, those funds are rewarded under normal operation but are held at pain of "slash" (expropriation) should they be found not to bee discharging their duties properly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Under deposit, those funds are rewarded" - Are the funds rewarded, or the maintainer? I think it should either say that the staked pot increases or the maintainer is rewarded, but rewarding funds is strange.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's settle for fund owner? though maintainer is also clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually think this is clear enough. This is the very frst section and using less keywords makes mroe sense. Funds are placed. Funds are rewarded. poeriod.
Of course, if anyone would read the full doc they would never confuse where the rewards are going.
In fact, funds can end of rewarding either the maintainer, or the fund intself (being being forwarded to the stash account directly).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about, "those funds' owners are rewarded..."
This actually makes it clear that,
- The funds can have multiple owners (the maintainer plus the nominators).
- It is the beneficiaries who are being rewarded, not the funds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Funds are placed. Funds are rewarded. poeriod.
It doesn't make sense to reward an inanimate object. To me, a reward implies gratification of some sort, and "funds" can't feel gratification, but their owners are happy to receive the additional funds.
srml/staking/src/lib.rs
Outdated
| //! | ||
| //! #### Staking | ||
| //! | ||
| //! Almost any interaction with the staking module requires at least one account to become **bonded**, also known as being a **staker**. For this, all that it is needed is a secondary _**stash account**_ which will hold the staked funds. Henceforth, the former account that initiated the interest is called the **controller** and the latter, holding the funds, is named the **stash**. Also, note that this implies that entering the staking process requires an _account pair_, one of which to take the role of the controller and one to be the frozen stash account (any value locked in stash cannot be used, hence called _frozen_). This process in the public API is mostly referred to as _bonding_ via the `bond()` function. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"one to take the role of the controller" (remove 'of which')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sounds ok to me but I am quite bad with formal English and trust that you have a reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't make sense because you have an account pair. There are two accounts, but one (singular) pair. To make it explicit, you could say, "one account to take..."
|
@joepetrowski @bkchr I believe all of your comments are addressed, except for the ones not collapsed where I've added sth to the discussion. |
srml/staking/src/lib.rs
Outdated
| //! # Staking Module | ||
| //! | ||
| //! <!-- Original author of paragraph: @gavofyork --> | ||
| //! The staking module is the means by which a set of network maintainers (known as "authorities" in some contexts and "validators" in others) are chosen based upon those who voluntarily place funds under deposit. Under deposit, those funds are rewarded under normal operation but are held at pain of "slash" (expropriation) should they be found not to bee discharging their duties properly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this under the "Overview" section header
We standardized the following line at the end of the overview section
For using the timestamp module you need to implement the following [Trait] (//links to the trait). Supported dispatchables are documented in the [Call] enum."
bkchr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small nits.
| //! are chosen based upon those who voluntarily place funds under deposit. Under deposit, those funds are rewarded under | ||
| //! normal operation but are held at pain of "slash" (expropriation) should the staked maintainer be found not to be | ||
| //! discharging their duties properly. | ||
| //! You can start using the Staking module by implementing the staking [`Trait`]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also link to the Call enum. See timestamp as an example.
srml/staking/src/lib.rs
Outdated
| //! | ||
| //! ## Interface | ||
| //! | ||
| //! ### Types |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We removed this in all the other documentations
|
Superseded by #2078 |
* WIP: SRML Example Module README * add newlines * review-fix: Change const to let. Explain generic usage more * refactor: Remove example steps 2 and 3. User can refer to other examples to figure it out * fix: Update to incorporate approved approach of staking module docs in PR #1951 * fix: Move into expandable Details arrow and fix syntax so appears correctly in rust docs * fix: Fix linting * docs: Add Public Dispatchable functions * fix: Rearrange to use Simple Code Snippet and Examples from SRML * fix: Remove duplicate Dispatchable Functions section * fix: Remove Implementation Details as requested by Gav
* WIP: SRML Example Module README * add newlines * review-fix: Change const to let. Explain generic usage more * refactor: Remove example steps 2 and 3. User can refer to other examples to figure it out * fix: Update to incorporate approved approach of staking module docs in PR paritytech#1951 * fix: Move into expandable Details arrow and fix syntax so appears correctly in rust docs * fix: Fix linting * docs: Add Public Dispatchable functions * fix: Rearrange to use Simple Code Snippet and Examples from SRML * fix: Remove duplicate Dispatchable Functions section * fix: Remove Implementation Details as requested by Gav
For this.
Notes:
What to do for ExtensibilityAdd links to a rustdoc once we have one?Some added, rest need a new update on rustdocMore/Other usage examples.