- Umbrela ticket: [[Staking] Approval Stake tracking #12719](https://github.com/paritytech/substrate/issues/12719) - Original PR: [Store validator approval-stakes in TargetList, use for ranking validators. #11013](https://github.com/paritytech/substrate/pull/11013) - [Polkadot Forum: Economic Model for System Parachains](https://forum.polkadot.network/t/economic-model-for-system-para-collators/1010/16) (very good discussion on the topic) ### Goals - 1. **Staking**: Implement a way to pre-sirt targets/validators in staking with bags-lists (`TargetList`), similar to current `VotersList` - 2. **Collator selection**: Implement a [parachain friendly election mechanism](https://forum.polkadot.network/t/economic-model-for-system-para-collators/1010) with approval voring. This requires the collators to be sorted by stake. Add an instance of the `pallet-bags-list`and `TargetsList` to the `pallet-collator-selection` to allow easy and fast collator validator selection The idea is to maintain the targets/validators pre-sorted by approval voting (staking) so that it's possible to perform an approval voting election on-chain. The sorting is performed and maintained through an instance of the [`pallet-bags-list`](https://paritytech.github.io/substrate/master/pallet_bags_list/). The sorted list of validators by stake can be used in different ways depending on the use case, e.g. in the collator pallet, it helps the parachain validators to be elected using an approval voting election scheme. ### Current state - [[Feature] Part 1: add TargetList for validator ranking #12034](https://github.com/paritytech/substrate/pull/12034) (PR merged) - [improve staking interface methods #14023](https://github.com/paritytech/substrate/pull/14023) (PR merged) - [remove `OnStakerSlash` replace with `OnStakingEvents` #14527](https://github.com/paritytech/substrate/pull/14527) (PR merged) ### Discussions - @Kian: Full branch that that I am breaking down is https://github.com/paritytech/substrate/tree/kiz-voter-list-stuff Next up from [@Kian](https://matrix.to/#/!OzILabrrLtoyLmmYbF:parity.io/$R1g1CxE8rmk7dDtjdSNcwXut-Kx_DwMKwsYGyxEAHJs?via=parity.io&via=matrix.org): 1. a refactor of `StakingLedger` that moves all operations that change the bonded amount to happen via helper functions 2. call into other events of `OnStakingUpdate` using the above, keep the current `Voterlist` updated using it, should be noop in production 3. merge `pallet-stake-tracker`, but don't add it to any runtime. The only piece I haven't coded yet is adding means of measuring total unstaking amount to this, which might come handy for real-fast-unstake. 4. migration, which should probably happen as an mbm. ### Next up - [x] Figure out current logic of `pallet-collator-selection` - [x] Understand current `pallet-collator-selection` design and start the dicussion with the SP team on implementation details - [x] Refactor `StakingLedger` - [x] ➡️ move `StakingLedger` to its own module - [x] consolidate all staking ledger changes in the new module - [x] update/improve the tests - [x] open PR - [x] Think about and design how to refactor the `staking_ledger` so that it's the main entry point in staking for all storage mutations/reads related to staking state per account. This should improve the code considerably and remove all the single calls into the `staking_ledger` happening in the code. - However, be careful with the extent of the refactoring to avoid breaking changes, complicated migrations (no MBMs yet), user/wallet-facing breaking changes and to keep in mind other parallel lines of work (controller deprecation, nomination pools in governance, reward pagination, etc) --- ### `pallet-collator-selection` and approval voting for collators - [System Parachain Collator Decentralization](https://github.com/paritytech/roadmap/issues/34) - [Add Simple Election for Permissionless Collators #2597](https://github.com/paritytech/cumulus/issues/2597) Pallet that manages the collators of a parachain. Main actors: - `Collator`: A parachain block producer; - `Bond`: An amount of `Balance` _reserved_ for candidate selection; (❓ no slashing, I believe) - `Invulnerable`: An account guaranteed to be in the collator set. The final collators list is aggregated from the `Invulnerables` (set by governance, always part of the final collator set) and `Candidates` (may or may not be selected to become a collator) storage items. Currently, the selection from the `Candidates` is done through a FIFO queue. The `Invulnerables` storage value (bounded vec) must be sorted. The goal is to perform the collator selection using approval voting implemented by selecting the top `N` candidates, to add to the `Invulnerables` as part of the final collator set. Collator candidates are set through a signed extrinsic `register_as_candidate` which should be called by the collator that wants to register. On the other hand, a new invulnerable is added through a specific origin (usually part of the parachain governance). `fn Pallet::assemble_collators`: assembles all the invulnerables and candidates into one vec, where the invulnerables are first and the candidates are extended. This the current "collator selection" logic. The pallet implements both the [`SessionManager`](https://paritytech.github.io/substrate/master/pallet_session/trait.SessionManager.html) trait, similar to the staking pallet, that returns a new validator set (`Option<Vec<T::AccountId>>`). The `fn assemble_collators` is called to provide the final result of the implementation of the `fn SessionManager::new_session` method. #### Questions/Todo - [x] ❓ how will the parachain "listen" to staking events through the `OnStakingEvents` interface to update the `TargetsList`/`bags-list`? - ✅ it doesn't, otherwise we'd have to implement restaking. with this implementation, the current bond of the collator candidates can be increase directly but the candidates themselves through a callable that reserves the bond. - [ ] add `try_state` to `pallet-collator-selection` - [x] ❓ what are the `Invulnerables` sorted by? and why? if by stake (I doubt it), should the invulnerables be part of the bags list as well, as far as we ensure they are always on top? - it seems the invulnerables are sorted by `AccountId`, why? (line 371, lib.rs) - ✅ Reason: to use `fn invulnerables.binary_search` when removing an account from the invulnerable set. the sorting doesn't matter in terms of the collator selection itself, since _all_ invulnerables will be part of final collator set. ### `SortedListProvider`, `pallet-bags-list` and approval voting selection - the trait `frame_election_provider_support::SortedListProvider` sets the interface a sorted list of some types. It is generic over an ID (most likely and `AccountId`) and it has an associated type `(Score`), which defines the metric the sorting metric. - the `pallet-bags-list` implements the `SortedListProvider` trait and it exposes the dispatchables to allow users to correct the list sorting. The pallet implements a semi(lazy)-sorted list that is efficient at item insertions and removals, as well as iterating over the top N items by score. --- ### Add `TargetList` to staking **Goals**: 1. **`TargetList`**: Implement the `TargetList` in the staking pallet (currently a noop) to keep a semi-sorted list of targets based on their approval voting; 2. **`staking_ledger`**: Refactor the `staking_ledger` logic to centralize all the mutating/reading logic related to bonds on the struct. #### 0. Design The goal is to call the `OnStakingUpdate` interface in the staking pallet whenever relevant (updates to stakers, bounds, nominations, etc). Both the voters and targets bag list instances will listen to the events triggered by staking and perform whatever local operations needed in their queues. - 💡 [future] it would be great if all the calls from nominations pools would be performed through staking and the nominations pools only would implement a `join` and `manage_pool` callables. The bond, withdraw, etc callables would be performed through the staking pallet, and the nomination pools pallet would implement the `OnStakingUpdate` interface. ![](https://hackmd.io/_uploads/HkQUQxnKn.png) The current design requires that there's a communication channel from `pallet-staking` to the instance of `pallet-bags-list`, where the this instance implements the `OnStakingUpdates` interface that can be called by the pallet staking when necessary. The voters lists don't need this because it is lazily sorted and it only needs to fetch the score from staking when a local callable is triggered. This can be implemented by adding a new pallet (`pallet-state-stracker`) and degree of indirection, which implements the `OnStakingUpdate` trait and that based on the staking update events, calls the correct target list `SortedListProvider::on_update` for the bags lists or any other pallet that is interested in listening to staking events. ![](https://hackmd.io/_uploads/rkbR_KhY3.png) - 💡 introduce a `SortedListProviderReadOnly` trait that only exposes the read methods from a sorted list provider. The trait should implement `SortedListProvider` and return `Self::Error` for all (potential) write operations (`on_update`, `on_decrease` etc..). - Both the `TargetsList` and `VotersList` associated types in the staking pallet should be of the type `SortedListProviderReadOnly`, since staking will use it only for reading the sorted list state. - The updates to the sorted list provider should be made through the `EventListners` associated type, which are propagated to the bags list sthrough the `pallet-state-tracker` (should this pallet be renamed to `pallet-staking-events-consumer` or similar?) #### 1. `TargetList` - A list that provides a best-effort sorted list of targets (electable validators). - Changes to the approval stake of each validator are reported to the `SortedListProvider` interface, namely any change to the stake of a validator/nominator, the role of the staker. - The values of this list are kept up to date through a `OnStakingUpdate`, which implements a generic staking event listener. ``` Event (e.g. nominate) -> OnStakingUpdate triggers -> TargetList update ``` - [x] ❓ why refactoring the `staking_ledger` is important for this line of work? - ✅ we want to centralize all the operations that will fire a `OnStakingUpdate` update; `Bonded` and `Ledger` reads/writes are all encapsulated under a clean API on `StakingLedger`. - [ ] ❓ list what is missing in the current implementation (many noops currently) - [ ] ❓ why is the `TargetList` so important in terms of security/stability? - perhaps to enable an on-chain approval voting mechanism when things go south? - [ ] ❓💡 attack vector: when a validator gets a lot of minimum nominations from sybil nominators in order to increase their position in the target list, which may be used for an on-chain approval voting based election (although the validator would unlikely be selected under npos). The current status is the following: - The `EventListeners` in staking is set to `pallet-nomination-pools`. - The nominations pool pallet implements the `OnStakingUpdate::on_slash` event. All the other events are noops (default). - The `pallet-staking` only fires `OnStakingUpdate::on_slash` events currently Requirements: - Target approvals are *always* up to date, i.e. the `TargetList` is updated when there are rewards and slashes; - This is done easily on the staking side once the `staking_ledger` refactor is merged, since the any update to the staking ledger will trigger an `on_stake_update` event (centralized, the call to update the stake through the `OnStakingUpdate` interface does not need to be all over the place). - [ ] ❓ how to distinguish rewards from nominators and from validators, though? we probably need to pass a `ledger.update(&who, fire_event)` to disable firing events when the nominator is being rewarded, since we don't want to propagate the rewards stake update to voterslist (although it would be better to trigger the event but being able to filter it on the `stake-tracker` side, which may not be possible without refactoring the `OnStakingLedger` trait) - [ ] ❓ how expensive is this, though? especially for large slashes -- should we do a simple multi-block update `on_slash` for targets and perhaps voters? - [ ] ⭐⭐ 💡 based on the above, I think we should have a multi-block event processing in the `stake-tracker` pallet and always maintain the voters and targets list up to date given slashes, rewards, etc. - are we fine with a few blocks update delay for targets and voters? **Todo** - [ ] add an indirection in `EventsListener` to `pallet-stake-tracker`, which listens to events from staking and then calls into correct listeners (`nomination-pools`, `bags-list-target-instance`, `bags-list-voters-instance`). - [ ] call into `OnStakingUpdate`: - [ ] `on_stake_update` - [ ] (perhaps) `on_validator_add` #### 2. `staking_ledger` PR with this refactor: https://github.com/paritytech/substrate/pull/14451 The staking pallet keep a storage map keyed by `AccountId` where each controller has an associated `StakingLedger` struct. The staking ledger keeps track of all bonded stash. ```rust /// The ledger of a (bonded) stash. #[derive(...)] #[scale_info(skip_type_params(T))] pub struct StakingLedger<T: Config> { /// The stash account whose balance is actually locked and at stake. pub stash: T::AccountId, /// The total amount of the stash's balance that we are currently accounting for. /// It's just `active` plus all the `unlocking` balances. #[codec(compact)] pub total: BalanceOf<T>, /// The total amount of the stash's balance that will be at stake in any forthcoming /// rounds. #[codec(compact)] pub active: BalanceOf<T>, /// Any balance that is becoming free, which may eventually be transferred out of the stash /// (assuming it doesn't get slashed first). It is assumed that this will be treated as a first /// in, first out queue where the new (higher value) eras get pushed on the back. pub unlocking: BoundedVec<UnlockChunk<BalanceOf<T>>, T::MaxUnlockingChunks>, /// List of eras for which the stakers behind a validator have claimed rewards. Only updated /// for validators. pub claimed_rewards: BoundedVec<EraIndex, T::HistoryDepth>, } ``` The problem seems to be that the staking ledger is updated all over the place in the staking pallet, by mutating directly the corresponding `StakingLedger` of the account which needs updating. This will inedviably spread all the `OnStakingUpdates` event triggering as well. Instead, the goal is to consolidate all the staking ledger mutations under the `StakingLedger` implementation, so that the `OnStakingUpdates` events will also be consolidated under that implementation. Currently, the pallet exposes a method `fn update_ledger(&AccountId, &StakingLedger<T>)` which mutates the staking ledger for a ledger entry of a specific account. This method should be moved under `StakingLedger` implementation and only be called there. ```rust /// Update the ledger for a controller. /// /// This will also update the stash lock. pub(crate) fn update_ledger(controller: &T::AccountId, ledger: &StakingLedger<T>) { T::Currency::set_lock(STAKING_ID, &ledger.stash, ledger.total, WithdrawReasons::all()); <Ledger<T>>::insert(controller, ledger); } ``` The main goal is to delete this method and perform all the lock setting and ledger mutations directly through `StakingLedger`. ⭐ Note (key insight, PR comments): the stash lock is updated _everytime the storage is mutated_ through the new method `StakingLedger::storage_put()` anf `StakingLedger::storage_remove()`. This means that setting and removing locks will be encapsulated under `StakingLedger`, which helps to avoid mistakes. (note, Kian's branch does the same). - [x] ❓ could ALL the lock management (`T::Currency::set_lock` and `T::Currency::remove_lock` ) be performed by the `StakingLedger`? - we will do it when `storage_put` (potentially on `storage_remove` too) - where else is the lock set? - `T::Currency::remove_lock`: - `Call::force_unstake`, paired with a `kill_stash` -> can be replaced with a call to `StakingLedger::storage_remove` - `Call::reap_stash`:, paired with a call to `kill_stash`, so it will be replaced with the call to `StakingLedger::storage_remove`. - `Pallet::do_withdraw_unbonded`, pairde with `kill_stash`, which will be replaced ith the sotrage remove call - [x] refactor code to encapsulate all the locks under `StakingLedger` impl (tests must pass). ⭐ Note (key insight, PR comments): another change is that all the mutations to `Ledger` now happen through `StakingInterface` to ensure that all the locks are set/updated/removed properly. - ⭐ `reads` *MUST* be done through `StakingLedger::<T>::get_storage` otherwise it won't be possible to store/remove the ledger entry correctly after fetched. - ⭐⭐ we need a warning or error mechanism when the controller is not available in the ledger entry - perhaps use and propagate errors, instead of using `unwrap` or `expect` in `StakingLedger::controller` #### explorations **Hide `Bond` storage item** Would it make sense to encapsulate `Ledger` and `Bond` structs? Eventually, the `Bond` will be deprecated (when the controller disappears). Perhaps the `Bond` data structure could be completely hidden from the code, so that the deprecation changes are minimal. All the calls would then happen via `StakingLedger` (which is what's happening in the future in any case). ⭐ (add to PR comments) A couple of things: we probably don't want to do anything here since eventually we will deprecate the controller account in staking and remove the `Bond` storage type altogether. Probably we should aim at leaving things as-is and build helpers that hide the `Bond` interface (e.g.) `StakingLedger::get_controller() -> T::AccountID`, which uses the underlying `Bond` storage item (too convoluted?) Where is `Bonded::<T>::*` used? - Get controller account from stash in `kill_stash`. The controller account is then used to remove the entry in the `Ledger` - In `Call::bond` to check whether a stash has been bonded - In `Call:set_controller` when setting the stash as its own controller (used as a deprecation mechanism) - In general, the `Bonded::<T>` map is used to fetch the controller from a stash account, in order to use as a key in the `Ledger` map (which maps controller accounts to the corresponding staking ledger). --- ### `stake-tracker` pallet and ledger refactoring integration PR