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 Payee and RewardDestination for split and controller removal #410

Open
rossbulat opened this issue Jun 22, 2023 · 1 comment
Open
Labels
I5-enhancement An additional feature request.

Comments

@rossbulat
Copy link

rossbulat commented Jun 22, 2023

This is an issue to track refactoring efforts of RewardDestination. The goals are two-fold:

  • To remove the Controller variant from RewardDestination enum.
  • To introduce a Split variant to compound a percentage of rewards, and send the remaining as free balance, if any, to a specified account.

The proposed solution that this issue will pursue is that of a lazy migration. There are around 69k Payee records in storage now, so a single block migration is not a viable proposition.

A proposed new enum,PayoutDestination, removes Controller, adds Split, and renames the variants to account for the fact that the stash & controller terminology is being deprecated in staking. This new enum will be used in a new Payees storage item.

What has come out of a Polkadot Forum discussion is also support for a simplified verb-based enum, that also uses Split to account for 100% of the payout to go to an account as free balance, without the need for a separate variant to do that.

#[derive(PartialEq, Eq, Copy, Clone, Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen)]
pub enum PayoutDestination<AccountId> {
  /// Pay into the stash account and add to bond.
  Stake,
  /// Pay the specified percentage to the specified account as free balance, and pay the rest, if
  /// any, into the stash account and add to bond. 0% and 100% should be disallowed and handled as
  /// `Stake` and `Free` respectively.
  Split((Perbill, AccountId)),
  /// Pay into an account as free balance.
  Deposit(AccountId),
  /// Receive no payout.
  Forgo,
}

Once all Payee records have been migrated to a new Payees storage item, we can confidently remove the former and replace RewardDestination with PayoutDestination throughout the codebase.

Migrating RewardDestination to PayoutDestination:

  • Stash -> Deposit(stash)
  • Controller -> Deposit(controller)
  • Account(AccountId) -> Deposit(account)
  • Staked -> Stake
  • None -> Forgo.

PR 1: Introduce new items, , update_payee to migrate to PayoutDestination / Payees.

paritytech/substrate#14451

[do lazy migration via update_payee]

PR 2:

  • Remove RewardDestination enum and Payee storage items.
  • Remove get_payout_destination and use storage getter.
@Polkadot-Forum
Copy link

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

https://forum.polkadot.network/t/revision-of-rewarddestination-to-account-for-split-and-controller-removal/3360/1

@juangirini juangirini transferred this issue from paritytech/substrate Aug 24, 2023
@the-right-joyce the-right-joyce added I5-enhancement An additional feature request. and removed J0-enhancement labels Aug 25, 2023
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
* add HeaderTimestamp associated type

* use Header Timestamp

* rename HeaderTimestamp to ChainTime

* add unit test

* deal with clippy

* Apply suggestions from code review

Commit review suggestions

Co-authored-by: Tomasz Drwięga <[email protected]>

* code review

* cargo fmt

* get rid of additional test runtime

* unit test asserts against concrete import context

Co-authored-by: Tomasz Drwięga <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
* add HeaderTimestamp associated type

* use Header Timestamp

* rename HeaderTimestamp to ChainTime

* add unit test

* deal with clippy

* Apply suggestions from code review

Commit review suggestions

Co-authored-by: Tomasz Drwięga <[email protected]>

* code review

* cargo fmt

* get rid of additional test runtime

* unit test asserts against concrete import context

Co-authored-by: Tomasz Drwięga <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
* add HeaderTimestamp associated type

* use Header Timestamp

* rename HeaderTimestamp to ChainTime

* add unit test

* deal with clippy

* Apply suggestions from code review

Commit review suggestions

Co-authored-by: Tomasz Drwięga <[email protected]>

* code review

* cargo fmt

* get rid of additional test runtime

* unit test asserts against concrete import context

Co-authored-by: Tomasz Drwięga <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
* add HeaderTimestamp associated type

* use Header Timestamp

* rename HeaderTimestamp to ChainTime

* add unit test

* deal with clippy

* Apply suggestions from code review

Commit review suggestions

Co-authored-by: Tomasz Drwięga <[email protected]>

* code review

* cargo fmt

* get rid of additional test runtime

* unit test asserts against concrete import context

Co-authored-by: Tomasz Drwięga <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
* add HeaderTimestamp associated type

* use Header Timestamp

* rename HeaderTimestamp to ChainTime

* add unit test

* deal with clippy

* Apply suggestions from code review

Commit review suggestions

Co-authored-by: Tomasz Drwięga <[email protected]>

* code review

* cargo fmt

* get rid of additional test runtime

* unit test asserts against concrete import context

Co-authored-by: Tomasz Drwięga <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
* add HeaderTimestamp associated type

* use Header Timestamp

* rename HeaderTimestamp to ChainTime

* add unit test

* deal with clippy

* Apply suggestions from code review

Commit review suggestions

Co-authored-by: Tomasz Drwięga <[email protected]>

* code review

* cargo fmt

* get rid of additional test runtime

* unit test asserts against concrete import context

Co-authored-by: Tomasz Drwięga <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
* add HeaderTimestamp associated type

* use Header Timestamp

* rename HeaderTimestamp to ChainTime

* add unit test

* deal with clippy

* Apply suggestions from code review

Commit review suggestions

Co-authored-by: Tomasz Drwięga <[email protected]>

* code review

* cargo fmt

* get rid of additional test runtime

* unit test asserts against concrete import context

Co-authored-by: Tomasz Drwięga <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
* add HeaderTimestamp associated type

* use Header Timestamp

* rename HeaderTimestamp to ChainTime

* add unit test

* deal with clippy

* Apply suggestions from code review

Commit review suggestions

Co-authored-by: Tomasz Drwięga <[email protected]>

* code review

* cargo fmt

* get rid of additional test runtime

* unit test asserts against concrete import context

Co-authored-by: Tomasz Drwięga <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
* add HeaderTimestamp associated type

* use Header Timestamp

* rename HeaderTimestamp to ChainTime

* add unit test

* deal with clippy

* Apply suggestions from code review

Commit review suggestions

Co-authored-by: Tomasz Drwięga <[email protected]>

* code review

* cargo fmt

* get rid of additional test runtime

* unit test asserts against concrete import context

Co-authored-by: Tomasz Drwięga <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
* add HeaderTimestamp associated type

* use Header Timestamp

* rename HeaderTimestamp to ChainTime

* add unit test

* deal with clippy

* Apply suggestions from code review

Commit review suggestions

Co-authored-by: Tomasz Drwięga <[email protected]>

* code review

* cargo fmt

* get rid of additional test runtime

* unit test asserts against concrete import context

Co-authored-by: Tomasz Drwięga <[email protected]>
jonathanudd pushed a commit to jonathanudd/polkadot-sdk that referenced this issue Apr 10, 2024
* add HeaderTimestamp associated type

* use Header Timestamp

* rename HeaderTimestamp to ChainTime

* add unit test

* deal with clippy

* Apply suggestions from code review

Commit review suggestions

Co-authored-by: Tomasz Drwięga <[email protected]>

* code review

* cargo fmt

* get rid of additional test runtime

* unit test asserts against concrete import context

Co-authored-by: Tomasz Drwięga <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I5-enhancement An additional feature request.
Projects
None yet
Development

No branches or pull requests

3 participants