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

staking: add manual_slash extrinsic #7805

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ordian
Copy link
Member

@ordian ordian commented Mar 5, 2025

This PR adds a convenience extrinsic manual_slash for the governance to slash a validator manually.

Changes

  • The on_offence implementation for the Staking pallet accepts a slice of OffenceDetails including the full validator exposure, however, it simply ignores that part. I've extracted the functionality into an inherent on_offence method that takes OffenceDetails without the full exposure and this is called directly in manual_slash
  • manual_slash creates an offence for a validator with a given slash percentange

Questions

  • should manual_slash accept session instead of an era when the validator was in the active set? staking thinks in terms of eras and we can check out of bounds this way, which is why it was chosen for this implementation, but if there are arguments against, happy to change to session index
  • should the accepted origin be something more than just root?
  • should I adapt this PR also against [DNM] Add AH-next runtime - the post-migration Asset Hub runtime #6996? looking at the changes, it should apply mostly without conflicts

@ordian ordian requested review from tdimitrov and Ank4n March 5, 2025 08:03
@ordian ordian added T1-FRAME This PR/Issue is related to core FRAME, the framework. T8-polkadot This PR/Issue is related to/affects the Polkadot network. labels Mar 5, 2025
@paritytech-workflow-stopper
Copy link

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/13671288111
Failed job name: test-linux-stable

/// will be applied.
/// - The slash will be deferred by `SlashDeferDuration` eras before being enacted.
#[pallet::call_index(33)]
#[pallet::weight(T::WeightInfo::force_unstake(1))]
Copy link
Member Author

Choose a reason for hiding this comment

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

TODO

Copy link
Contributor

@tdimitrov tdimitrov left a comment

Choose a reason for hiding this comment

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

Looks good. Approving modulo the question I've left and your todo.

My view on the questions in the PR description:

should manual_slash accept session instead of an era when the validator was in the active set?

I can't think about a case when we want to slash at a particular session so I'd say this is good.

should the accepted origin be something more than just root?

Maybe we might want to apply a manual slash from AH via XCM, but I wouldn't worry about this right now. I think it's good as it is.

should I adapt this PR also against #6996?

@Ank4n and @kianenigma are doing a lot of changes to the staking pallet here which will be merged in master. I think having your PR merged in master is enough.

@@ -2639,5 +2639,63 @@ pub mod pallet {

Ok(())
}

/// This function allows governance to manually slash of a validator and is a
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// This function allows governance to manually slash of a validator and is a
/// This function allows governance to manually slash a validator and is a


// Check era is valid
let current_era = CurrentEra::<T>::get().ok_or(Error::<T>::InvalidEraToReward)?;
let history_depth = T::HistoryDepth::get();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a point to check if validator_stash was in the active set during era or this is performed later when handling the offence?

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. T8-polkadot This PR/Issue is related to/affects the Polkadot network.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants