Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor staking ledger #1484

Merged
merged 4 commits into from
Oct 15, 2023
Merged

Refactor staking ledger #1484

merged 4 commits into from
Oct 15, 2023

Conversation

gpestana
Copy link
Contributor

Moves paritytech/substrate#14582 into polkadot-sdk


This PR refactors the staking ledger logic to encapsulate all reads and mutations of Ledger, Bonded, Payee and stake locks within the StakingLedger struct implementation.

With these changes, all the reads and mutations to the Ledger, Payee and Bonded storage map should be done through the methods exposed by StakingLedger to ensure the data and lock consistency of the operations. The new introduced methods that mutate and read Ledger are:

  • ledger.update(): inserts/updates a staking ledger in storage; updates staking locks accordingly (and ledger.bond(), which is synthatic sugar for ledger.update())
  • ledger.kill(): removes all Bonded and StakingLedger related data for a given ledger; updates staking locks accordingly;
    StakingLedger::get(account): queries both the Bonded and Ledger storages and returns a Option<StakingLedger>. The pallet impl exposes fn ledger(account) as synthatic sugar for StakingLedger::get(account).

Retrieving a ledger with StakingLedger::get() can be done by providing either a stash or controller account. The input must be wrapped in a StakingAccount variant (Stash or Controller) which is treated accordingly. This simplifies the caller API but will eventually be deprecated once we completely get rid of the controller account in staking. However, this refactor will help with the work necessary when completely removing the controller.

Other goals:

  • No logical changes have been introduced in this PR;
  • No breaking changes or updates in wallets required;
  • No new storage items or need to perform storage migrations;
  • Centralise the changes to bonds and ledger updates to simplify the OnStakingUpdate updates to the target list (related to [Staking] Approval Stake tracking #443)

Note: it would be great to prevent or at least raise a warning if Ledger<T>, Payee<T> and Bonded<T> storage types are accessed outside the StakingLedger implementation. This PR should not get blocked by that feature, but there's a tracking issue here #149

Related and step towards #443

This PR refactors the staking ledger logic to encapsulate all reads and mutations of `Ledger`, `Bonded`, `Payee` and
stake locks within the `StakingLedger` struct implementation.

Lift and shift from paritytech/substrate#14582
@gpestana gpestana requested a review from Ank4n September 11, 2023 01:19
@gpestana gpestana self-assigned this Sep 11, 2023
@gpestana gpestana requested review from a team September 18, 2023 12:00
@gpestana gpestana added T1-FRAME This PR/Issue is related to core FRAME, the framework. T2-pallets This PR/Issue is related to a particular pallet. labels Sep 18, 2023
Copy link
Contributor

@Ank4n Ank4n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really awesome work with the refactor. 😇

@gpestana gpestana merged commit 8ee4042 into master Oct 15, 2023
8 checks passed
@gpestana gpestana deleted the gpestana/staking-ledger-ref branch October 15, 2023 20:50
ordian added a commit that referenced this pull request Oct 16, 2023
* master: (54 commits)
  Publish `xcm-emulator` crate (#1881)
  Adding migrations to clean Rococo Gov 1 storage & reserved funds (#1849)
  Arkworks Elliptic Curve utils overhaul (#1870)
  Fix typos (#1878)
  fix: GoAhead signal only set when runtime upgrade is enacted from parachain side (#1176)
  Refactor staking ledger (#1484)
  Paired-key Crypto Scheme (#1705)
  Include polkadot version in artifact path (#1828)
  add link to rfc-0001 in broker README (#1862)
  Discard `Executor` (#1855)
  Macros to use path instead of ident (#1474)
  Remove clippy clone-double-ref lint noise (#1860)
  Refactor alliance benchmarks to v2 (#1868)
  Check executor params coherence (#1774)
  frame: use derive-impl for beefy and mmr pallets (#1867)
  sc-consensus-beefy: improve gossip logic (#1852)
  Adds instance support for composite enums (#1857)
  Fix links to implementers' guide (#1865)
  Disabled validators runtime API (#1257)
  Adding `try_state` hook for `Treasury` pallet (#1820)
  ...
ordian added a commit that referenced this pull request Oct 16, 2023
…ribution

* tsv-disabling-backing: (54 commits)
  Publish `xcm-emulator` crate (#1881)
  Adding migrations to clean Rococo Gov 1 storage & reserved funds (#1849)
  Arkworks Elliptic Curve utils overhaul (#1870)
  Fix typos (#1878)
  fix: GoAhead signal only set when runtime upgrade is enacted from parachain side (#1176)
  Refactor staking ledger (#1484)
  Paired-key Crypto Scheme (#1705)
  Include polkadot version in artifact path (#1828)
  add link to rfc-0001 in broker README (#1862)
  Discard `Executor` (#1855)
  Macros to use path instead of ident (#1474)
  Remove clippy clone-double-ref lint noise (#1860)
  Refactor alliance benchmarks to v2 (#1868)
  Check executor params coherence (#1774)
  frame: use derive-impl for beefy and mmr pallets (#1867)
  sc-consensus-beefy: improve gossip logic (#1852)
  Adds instance support for composite enums (#1857)
  Fix links to implementers' guide (#1865)
  Disabled validators runtime API (#1257)
  Adding `try_state` hook for `Treasury` pallet (#1820)
  ...
HCastano added a commit to entropyxyz/entropy-core that referenced this pull request Jan 19, 2024
Starts introducing some of the changes brought in by
paritytech/polkadot-sdk#1484.
HCastano added a commit to entropyxyz/entropy-core that referenced this pull request Jan 23, 2024
* Get `entropy-shared` compiling with updated deps

* Fix `entropy-shared` build for `wasm-no-std`

* Make `pallet-staking-extension` compile

Starts introducing some of the changes brought in by
paritytech/polkadot-sdk#1484.

* Introduce staking changes from paritytech/substrate#12970

* Add RuntimeFreezeReason from paritytech/polkadot-sdk#1900

* Re-map errors in staking extension pallet

* Bump `pallet-programs`

* Missed a crate dep for staking extension

* Remove commented out code

* Get `pallet-relayer` compiling

* Fix relayer test compilation

* Fix `pallet-free-tx`'s tests

* Reorder config types

* Bump `pallet-propagation` dependencies

* Bump `pallet-slashing` dependencies

* Bump `pallet-transaction-pause`

This includes the deprecation of `Balances::transfer` from
paritytech/polkadot-sdk#1226.

* TaploFmt

* Use `StakingAccount` instead of `.into()`

* Import `vec` macro in `pallet-slashing`

* Import `vec` in other pallets

* Bump TOML dependencies for runtime

Haven't fixed the Rust code yet though

* Self define types instead of using `node-primitives`

* Update staking related configs

Values were mostly taken from the Polkadot runtime config

* Update `pallet-preimage` config

Changes introduced in paritytech/polkadot-sdk#1363

* Add `MaxNominators` to `pallet_babe`

Introduced in paritytech/substrate#14471

* Add `MaxNominators` to `pallet_grandpa`

Also introduced in paritytech/substrate#14471

* Add `RuntimeFreezeReason` to `pallet_nomination_pools`

* Add `MaxTipAmount` to `pallet_tips`

Introduced in paritytech/polkadot-sdk#1709

* Add `IdentityInformation` to `pallet_identity`

Introduced in paritytech/polkadot-sdk#1661

* finish runtime

* Use `crates.io` packages in `entropy` TOML

This doesn't compile yet

* Remove reference to `node_primitives`

* Move imports under correct dependency section

These shouldn't be built-deps, my bad

* Change `WarpSyncParams` import to come from `sc_network_sync`

Change introduced here: paritytech/polkadot-sdk#1912

* Add missing `sc-consensus-babe` package

* Remove second generic from `DefaultImportQueue`

This was removed in paritytech/substrate#14612

* Add temporary cursed RPC bounds

* Switch `sp-consensus-grandpa` to be from Crates

* Add Grandpa justification period to service

* Rename `justification_period to` `justification_generation_period`

* Add missing `block_relay` param

Introduced in paritytech/polkadot-sdk#1524

* Try to clean up runtime types

* Add `opaque` types to runtime

These match what was in `node-primitives`. In particular we're interested in the `Block` type which
we were using in a lot of places, and I incorrectly changed to be `Unchechecked` instead of
`Opaque`.

* Use opaque block type in service

* Add full session key impls

* Remove cursed trait bounds for RPC

* Convert some tabs to spaces

Sorry, I had to

* Import `Vec` from `sp_std` in benchmarks

Turns out the issue responsible for this change is this:
paritytech/polkadot-sdk#172

* Pull `TrackedStorageKey` from `sp_storage`

From here: paritytech/substrate#14787

* Add `BenchmarkHelper` to treasury pallet

* TaploFmt

* change max freezes

* Sort TOML imports in runtime manifest

* Move `version` field to be first in runtime manifest

* Organize imports in pallet manifests

* Sort and organize imports for client

* Address TODOs in `entropy-shared`

* Remove TODOs related to `node-primitives`

* Pull `substrate-wasm-builder` from crates.io

* TaploFmt

* Fix benchmarking import post-merge

* Fix another program import port-merge

* Import `Vec` to programs benches

* Wrong vec import 🙃

* Poke CI

* Bump node metadata

---------

Co-authored-by: Jesse Abramowitz <[email protected]>
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
This PR refactors the staking ledger logic to encapsulate all reads and
mutations of `Ledger`, `Bonded`, `Payee` and stake locks within the
`StakingLedger` struct implementation.

With these changes, all the reads and mutations to the `Ledger`, `Payee`
and `Bonded` storage map should be done through the methods exposed by
StakingLedger to ensure the data and lock consistency of the operations.
The new introduced methods that mutate and read Ledger are:

- `ledger.update()`: inserts/updates a staking ledger in storage;
updates staking locks accordingly (and ledger.bond(), which is synthatic
sugar for ledger.update())
- `ledger.kill()`: removes all Bonded and StakingLedger related data for
a given ledger; updates staking locks accordingly;
`StakingLedger::get(account)`: queries both the `Bonded` and `Ledger`
storages and returns a `Option<StakingLedger>`. The pallet impl exposes
fn ledger(account) as synthatic sugar for `StakingLedger::get(account)`.

Retrieving a ledger with `StakingLedger::get()` can be done by providing
either a stash or controller account. The input must be wrapped in a
`StakingAccount` variant (Stash or Controller) which is treated
accordingly. This simplifies the caller API but will eventually be
deprecated once we completely get rid of the controller account in
staking. However, this refactor will help with the work necessary when
completely removing the controller.

Other goals:

- No logical changes have been introduced in this PR;
- No breaking changes or updates in wallets required;
- No new storage items or need to perform storage migrations;
- Centralise the changes to bonds and ledger updates to simplify the
OnStakingUpdate updates to the target list (related to
paritytech#443)

Note: it would be great to prevent or at least raise a warning if
`Ledger<T>`, `Payee<T>` and `Bonded<T>` storage types are accessed
outside the `StakingLedger` implementation. This PR should not get
blocked by that feature, but there's a tracking issue here
paritytech#149

Related and step towards
paritytech#443
bkchr pushed a commit that referenced this pull request Apr 10, 2024
* parachain loop metrics

* some fixes

* mini refactoring

* add tests
@Polkadot-Forum
Copy link

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

https://forum.polkadot.network/t/recover-corrupted-staking-ledgers-in-polkadot-and-kusama/9796/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T1-FRAME This PR/Issue is related to core FRAME, the framework. T2-pallets This PR/Issue is related to a particular pallet.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants