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

Refactors the OnStakingUpdate trait #2839

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions prdoc/pr_2839.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
title: "Refactors `OnStakingUpdate` trait"

doc:
- audience: Runtime Dev
description: Refactors the `sp_staking:OnStakingUpdate` to make event inputs more explicit.

crates:
- name: "sp-staking"
20 changes: 15 additions & 5 deletions substrate/primitives/staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,19 +100,29 @@ pub trait OnStakingUpdate<AccountId, Balance> {
///
/// This is effectively any changes to the bond amount, such as bonding more funds, and
/// unbonding.
fn on_stake_update(_who: &AccountId, _prev_stake: Option<Stake<Balance>>) {}
fn on_stake_update(
_who: &AccountId,
_prev_stake: Option<Stake<Balance>>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we have both _prev_stake and _stake as same type? Either Stake<Balance> or Option<Stake<Balance>>. If Stake<Balance>, we can pass a zero value.

_stake: Stake<Balance>,
) {
}

/// Fired when someone sets their intention to nominate.
///
/// This should never be fired for existing nominators.
fn on_nominator_add(_who: &AccountId) {}
fn on_nominator_add(_who: &AccountId, _nominations: Vec<AccountId>) {}

/// Fired when an existing nominator updates their nominations.
///
/// Note that this is not fired when a nominator changes their stake. For that,
/// `on_stake_update` should be used, followed by querying whether `who` was a validator or a
/// nominator.
fn on_nominator_update(_who: &AccountId, _prev_nominations: Vec<AccountId>) {}
fn on_nominator_update(
_who: &AccountId,
_prev_nominations: Vec<AccountId>,
_nominations: Vec<AccountId>,
) {
}

/// Fired when someone removes their intention to nominate, either due to chill or validating.
///
Expand All @@ -123,15 +133,15 @@ pub trait OnStakingUpdate<AccountId, Balance> {
/// Fired when someone sets their intention to validate.
///
/// Note validator preference changes are not communicated, but could be added if needed.
fn on_validator_add(_who: &AccountId) {}
fn on_validator_add(_who: &AccountId, _self_stake: Option<Stake<Balance>>) {}

/// Fired when an existing validator updates their preferences.
///
/// Note validator preference changes are not communicated, but could be added if needed.
fn on_validator_update(_who: &AccountId) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't this also need old and new self stake params?


/// Fired when someone removes their intention to validate, either due to chill or nominating.
fn on_validator_remove(_who: &AccountId) {}
fn on_validator_remove(_who: &AccountId, _self_stake: Option<Stake<Balance>>) {}

/// Fired when someone is fully unstaked.
fn on_unstake(_who: &AccountId) {}
Expand Down