[AHM] Revert multi-block election and slashing from staking-classic #7939

Ank4n · 2025-03-16T23:47:35Z

Context

We are forking pallet-staking into pallet-staking (also referred as staking-classic, this is the version that will stay on RC) and pallet-staking-next which will live on AH post AHM.

Reverted PRs

Westend Migration

pallet-staking on Westend migrated to v17 with the following changes

New:
- OffenceQueue (StorageDoubleMap)
  - K1: Era
  - K2: Offending validator account
  - V: OffenceRecord
- OffenceQueueEras (StorageValue)
  - V: BoundedVec<EraIndex, BoundingDuration>
- ProcessingOffence (StorageValue)
  - V: (Era, offending validator account, OffenceRecord)
Changed:
- UnappliedSlashes:
  - Old: StorageMap<K -> Era, V -> Vec<UnappliedSlash>>
  - New: StorageDoubleMap<K1 -> Era, K2 -> (validator_acc, perbill, page_index), V -> UnappliedSlash>

To revert these changes on Westend, since this is a testnet, we can simply drop any keys if present associated with the above storages, and reset storage version of pallet-staking to 16.

For AHM migration

The UnappliedSlashes storage will need to be translated from rc::staking-classic to ah::staking-next. Bookmarking the code that can be referred for this.

TODO

Reset storage version to 16 for pallet-staking in Westend.
Re-bench
Remove exposure dependency on RC as a followup to this PR: [AHM] Replace Validator FullIdentification from Exposure to Existence #7936.

)" This reverts commit 669b889.

…ash Application (#7424)" This reverts commit dda2cb5.

This reverts commit a025562.

Ank4n · 2025-03-17T00:02:25Z

polkadot/runtime/westend/src/lib.rs

@@ -1874,7 +1863,6 @@ pub mod migrations {
 		parachains_shared::migration::MigrateToV1<Runtime>,
 		parachains_scheduler::migration::MigrateV2ToV3<Runtime>,
 		pallet_staking::migrations::v16::MigrateV15ToV16<Runtime>,
-		pallet_staking::migrations::v17::MigrateV16ToV17<Runtime>,


In westend this has already been applied, but we can just sudo kill any storage keys created (if any). If there are any slashes it will be dropped, but for the testnet that's okay.

paritytech-workflow-stopper · 2025-03-17T16:21:06Z

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

kianenigma · 2025-03-17T20:51:33Z

substrate/bin/node/cli/Cargo.toml

@@ -182,9 +182,6 @@ try-runtime = [
 	"polkadot-sdk/try-runtime",
 	"substrate-cli-test-utils/try-runtime",
 ]
-staking-playground = [


yeah, glad to see this stuff reverted. I now have a better dual rutnime setup to test staking-next.

kianenigma · 2025-03-17T20:53:52Z

substrate/bin/node/runtime/src/genesis_config_presets.rs

@@ -58,7 +58,7 @@ pub fn kitchensink_genesis(
 	stakers: Vec<Staker>,
 	staking_playground_config: Option<StakingPlaygroundConfig>,
 ) -> serde_json::Value {
-	let (validator_count, min_validator_count, dev_stakers) = match staking_playground_config {
+	let (validator_count, min_validator_count, _dev_stakers) = match staking_playground_config {


hmm I don't remember this code, or how it has gotten to master :))

I thought we would revert all things related to this taking playground idea in substrate node.

yeah it was done here:

https://github.com/paritytech/polkadot-sdk/pull/7741/files#diff-098d0b6ca72ffb6df6091f5b5433fa7131326bd062e911133e12ce323213dd58R318

Since pallet-staking has no dev-stakers, you should remove that. The rest is harmless.

kianenigma · 2025-03-17T20:58:52Z

substrate/frame/bags-list/src/benchmarks.rs

@@ -29,125 +29,6 @@ use frame_system::RawOrigin as SystemOrigin;
 use sp_runtime::traits::One;

 benchmarks_instance_pallet! {
-	// iteration of any number of items should only touch that many nodes and bags.


These are useful, we can keep them.

kianenigma · 2025-03-17T21:03:55Z

substrate/frame/election-provider-support/solution-type/src/single_page.rs

@@ -84,8 +84,6 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result<TokenStream2> {
 			Eq,


Please undo this revert, it is harmless elsewhere and needed later.

kianenigma · 2025-03-17T21:04:02Z

substrate/frame/election-provider-support/solution-type/src/codec.rs

@@ -33,7 +33,6 @@ pub(crate) fn codec_and_info_impl(
 	let scale_info = scale_info_impl(&ident, &voter_type, &target_type, &weight_type, count);

 	quote! {
-		impl _fepsp::codec::EncodeLike for #ident {}


Can be kept

kianenigma · 2025-03-17T21:05:03Z

substrate/frame/election-provider-support/src/bounds.rs

@@ -155,15 +154,6 @@ impl DataProviderBounds {
 			self.size_exhausted(given_size.unwrap_or(SizeBound::zero()))
 	}

-	/// Ensures the given encode-able slice meets both the length and count bounds.


also seems useful and harmless to keep

kianenigma · 2025-03-17T21:07:45Z

substrate/frame/election-provider-support/src/lib.rs

-	<E as ElectionProvider>::MaxWinnersPerPage,
-	<E as ElectionProvider>::MaxBackersPerWinner,
+	<E as ElectionProviderBase>::AccountId,
+	<E as ElectionProviderBase>::MaxWinners,


Okay, at first I was against this revert.

My idea was that we keep these traits multi-page + capable of being bounded (having things like MaxWinners etc), but now I now see that this can cause too much headache to get this revert done.

This will totally fuck me when I merge master, as I do need to bring a lot of it back, but I think it can be useful.

pallet-staking + pallet-epm + frame-ep-support atm work together, and they are: unbounded, and single page

I will add a new mod multi_page here later, which would bring in the traits needed for bounded + multi-page support.

A bit redundant code, but I can see now that it is a safe choice.

but, I think we could have also kept the current version. I don't think there is time for it now anymore though..

kianenigma · 2025-03-17T21:11:44Z

substrate/frame/election-provider-support/src/traits.rs

@@ -127,23 +123,4 @@ where
 		voter_at: impl Fn(Self::VoterIndex) -> Option<A>,
 		target_at: impl Fn(Self::TargetIndex) -> Option<A>,
 	) -> Result<Vec<Assignment<A, Self::Accuracy>>, Error>;
-
-	/// Sort self by the means of the given function.


Same, please undo the revert here. 100% harmless and just less merge aissues later.

kianenigma · 2025-03-17T21:13:45Z

substrate/frame/session/benchmarking/src/inner.rs

@@ -58,7 +58,6 @@ mod benchmarks {
 			false,
 			true,
 			RewardDestination::Staked,
-			pallet_staking::CurrentEra::<T>::get().unwrap_or_default(),


A follow-up: we need to re-benchmark the session pallet. Atm the benchmarking is tightly coupled with pallet-staking.

Can you please make an issue for this and add to our board?

kianenigma · 2025-03-17T21:17:35Z

substrate/primitives/npos-elections/src/helpers.rs

 	}
 	Ok(ratio)
 }

-/// Convert some [`Supports`]s into vector of [`StakedAssignment`]
-pub fn supports_to_staked_assignment<A: IdentifierT>(


Also useful, please keep.

kianenigma · 2025-03-17T21:19:01Z

substrate/primitives/npos-elections/src/lib.rs


-	ElectionScore { minimal_stake, sum_stake, sum_stake_squared }
+		ElectionScore { minimal_stake, sum_stake, sum_stake_squared }
+	}


Similarly, this file can be kept in the new format.

kianenigma · 2025-03-17T21:19:58Z

substrate/primitives/staking/src/lib.rs

@@ -523,42 +486,6 @@ pub struct PagedExposureMetadata<Balance: HasCompact + codec::MaxEncodedLen> {
 	pub page_count: Page,
 }

-impl<Balance> PagedExposureMetadata<Balance>


I am surprised we don't need these things? @Ank4n you would know better, they are related to paged exposures.

kianenigma · 2025-03-17T21:20:34Z

substrate/utils/frame/benchmarking-cli/src/pallet/command.rs

-			include == "*" ||
-			include == "all" ||
-			include.as_bytes() == pallet;
+		let included = include.is_empty() || include == "*" || include.as_bytes() == pallet;


can be kept, it is quite useful for benchmarking.

kianenigma

Okay, as noted, my origina, idea was to keep the additional traits and types in frame-election-provider-support, but I am okay to fuly revert them.

I will re-create them from the staking-next PR, as new traits, leaving everything as-is.

I have pointed out a few changes to sp-npos-elections and solution-type macro that can stay and are harmless.

A bit of a compromise, but since we need this by end of month, happy to go down this path.

Ank4n added 8 commits March 16, 2025 22:43

Revert "Implementation of ah-client and rc-client staking pallets (#7582

e1629e7

)" This reverts commit 669b889.

Revert "[Staking] Bounded Slashing: Paginated Offence Processing & Sl…

82513b0

…ash Application (#7424)" This reverts commit dda2cb5.

Revert "[AHM] Multi-block staking election pallet (#7282)"

1ecb0c4

This reverts commit a025562.

test compiles, 17 failure

dcddac6

ensure offence is reported to session

d4364a4

fmt

42a0b9d

fix westend

e14b22b

unrevert some stuff that we need

c15fb37

Ank4n commented Mar 17, 2025

View reviewed changes

Ank4n added 2 commits March 17, 2025 01:55

bring back some more changes

e6fc0c1

fmt

755aacb

Ank4n added I4-refactor T2-pallets labels Mar 17, 2025

Ank4n marked this pull request as ready for review March 17, 2025 06:42

Ank4n requested review from acatangiu and a team as code owners March 17, 2025 06:42

tdimitrov mentioned this pull request Mar 17, 2025

[AHM] Revert some AH-next staking changes from master #7937

Closed

3 tasks

Ank4n and others added 11 commits March 17, 2025 08:47

fix manual slash test

667d059

fix some more tests

781e565

Fix the failing staking tests

d9ccb91

Merge branch 'master' into ankn/staking-classic

2721167

Fix a compilation error in benchmarks

3ce2484

Fix a compilation warning

f3c83b7

fix bench

82d58e3

fix root offences

0f7a0f4

fix kitchensink genesis

aaa9fea

taplo

e9a04a6

update storage version

202138e

Ank4n changed the title ~~Revert the multiblock changes to staking~~ [AHM] Revert multi-block election and slashing from staking-classic Mar 17, 2025

Ank4n requested review from kianenigma, ggwpez, seadanda and tdimitrov March 17, 2025 14:58

node cli

e80b05c

kianenigma reviewed Mar 17, 2025

View reviewed changes

kianenigma approved these changes Mar 17, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AHM] Revert multi-block election and slashing from staking-classic #7939

[AHM] Revert multi-block election and slashing from staking-classic #7939

Ank4n commented Mar 16, 2025 •

edited

Loading

Ank4n Mar 17, 2025

paritytech-workflow-stopper bot commented Mar 17, 2025

kianenigma Mar 17, 2025

kianenigma Mar 17, 2025

kianenigma Mar 17, 2025

kianenigma Mar 17, 2025

kianenigma Mar 17, 2025

kianenigma Mar 17, 2025

kianenigma Mar 17, 2025

kianenigma Mar 17, 2025

kianenigma Mar 17, 2025

kianenigma Mar 17, 2025

kianenigma Mar 17, 2025

kianenigma Mar 17, 2025

kianenigma Mar 17, 2025

kianenigma Mar 17, 2025

kianenigma Mar 17, 2025

kianenigma left a comment

		@@ -84,8 +84,6 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result<TokenStream2> {
		Eq,

[AHM] Revert multi-block election and slashing from staking-classic #7939

Are you sure you want to change the base?

[AHM] Revert multi-block election and slashing from staking-classic #7939

Conversation

Ank4n commented Mar 16, 2025 • edited Loading

Context

Reverted PRs

Westend Migration

For AHM migration

TODO

Choose a reason for hiding this comment

paritytech-workflow-stopper bot commented Mar 17, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kianenigma left a comment

Choose a reason for hiding this comment

Ank4n commented Mar 16, 2025 •

edited

Loading