[AHM] Replace Validator FullIdentification from `Exposure` to `Existence` #7936

Ank4n · 2025-03-15T19:24:19Z

closes #6344.

TODO

TryStateCheck to see existing offence report decodes correctly.

Ank4n · 2025-03-15T19:30:07Z

/cmd prdoc --force

paritytech-workflow-stopper · 2025-03-15T21:51:18Z

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

kianenigma · 2025-03-16T09:43:00Z

substrate/frame/staking/src/lib.rs

-	fn convert(_: T) -> Option<()> {
-		Some(())
+/// A marker type representing the presence of a validator. Encodes as a unit type.
+pub type Existence = ();


fwiw this is a type alias to (), what we usually call a marker type is actually a different type, which doesn't inherit any of the impls of the original type, such as pub struct Existence

Ank4n · 2025-03-16T10:20:06Z

substrate/frame/staking/src/lib.rs

+/// Encodes as type [`Existence`] but can decode from legacy [`Exposure`] values for backward
+/// compatibility.
+#[derive(PartialEq, Eq, PartialOrd, Ord, Clone, RuntimeDebug, TypeInfo, DecodeWithMemTracking)]
+pub enum ExistenceOrLegacyExposure<A, B: HasCompact> {


Could have just used Option but this is more readable.

kianenigma · 2025-03-16T10:22:52Z

substrate/frame/staking/src/lib.rs

+
+/// A converter type that returns `Some(())` if the validator exists in the current active era,
+/// otherwise `None`. This serves as a lightweight presence check for validators.
+pub struct ExistenceOf<T>(core::marker::PhantomData<T>);


We should move this to stkaing-next branch, right?

staking-classic only needs ExistenceOrLegacyExposureOf.

We need them only in staking-classic, right? Staking-next does not interact with historical sessions atleast directly?

We don't strictly need ExistenceOf. But seemed a nice thing to have. New solo chains can choose ExistenceOf instead of ExistenceOrLegacyExposureOf .

kianenigma · 2025-03-16T10:24:27Z

substrate/frame/staking/src/pallet/impls.rs

@@ -1775,12 +1756,12 @@ impl<T: Config> historical::SessionManager<T::AccountId, Exposure<T::AccountId,
 	}
 }

-impl<T: Config> historical::SessionManager<T::AccountId, ()> for Pallet<T> {
-	fn new_session(new_index: SessionIndex) -> Option<Vec<(T::AccountId, ())>> {
+impl<T: Config> historical::SessionManager<T::AccountId, Existence> for Pallet<T> {


Similarly, this code will be removed once we revert #7937

I can get rid of them here as well. They are used by the test runtimes.

kianenigma

Should wait for #7937 before being merged, otherwise LGTM.

kianenigma · 2025-03-16T10:26:52Z

substrate/frame/staking/src/lib.rs

+		assert!(matches!(
+			decoded_legacy_exposure,
+			ExistenceOrLegacyExposure::Exposure(Exposure {
+				total: 1,
+				own: 2,
+				others: ref i
+			}) if *i == vec![IndividualExposure { who: 3, value: 4 }]
+		));


Suggested change

assert!(matches!(

decoded_legacy_exposure,

ExistenceOrLegacyExposure::Exposure(Exposure {

total: 1,

own: 2,

others: ref i

}) if *i == vec![IndividualExposure { who: 3, value: 4 }]

));

assert_eq!(

decoded_legacy_exposure,

ExistenceOrLegacyExposure::Exposure(Exposure {

total: 1,

own: 2,

others: vec![IndividualExposure { who: 3, value: 4 }]

}

));

nit: I think marches! gives worst errors messages + is harder to write and read, unless if you want to skip some fields, which makes it worthwhile.

kianenigma · 2025-03-16T10:28:03Z

substrate/frame/staking/src/lib.rs

+	}
+
+	#[test]
+	fn legacy_existence_encodes_decodes_correctly() {


Also you are missing a test to make sure () can be decoded as ExistenceOrLegacyExposure

kianenigma · 2025-03-16T10:28:35Z

substrate/frame/staking/src/lib.rs

+
+impl<A, B: HasCompact> MaxEncodedLen for ExistenceOrLegacyExposure<A, B> {
+	fn max_encoded_len() -> usize {
+		0


Suggested change

0

Exposure::<A, B>::max_encoded_len()

kianenigma · 2025-03-16T10:29:54Z

substrate/frame/staking/src/lib.rs

+}
+
+impl<A, B: HasCompact> Encode for ExistenceOrLegacyExposure<A, B> {
+	fn encode_to<T: Output + ?Sized>(&self, _: &mut T) {}


Should encode it the same as Exposure<A, B> right?

Encoding still has issues.

AFAIK ExistenceOrLegacyExposure should:

Encode equal to the old Exposure
Decode both into Existence, or itself.

Namely, such that if any historical offences come in with the old proof that contains exposure, it can be decoded.

Any new offence will be generated with Existence

iulianbarbu · 2025-03-16T19:17:28Z

#j uky lgsn gtop

Ank4n added 4 commits March 15, 2025 18:37

create the types for existence as identification

8619582

add test

5c8b2bc

fmt

0be27ce

full identification backward compatible

e043ded

Ank4n changed the title ~~Change full identification of Validator from its exposure to just existence~~ [AHM] Replace Validator FullIdentification from Exposure to Existence Mar 15, 2025

Ank4n added I4-refactor Code needs refactoring. T8-polkadot This PR/Issue is related to/affects the Polkadot network. labels Mar 15, 2025

Merge branch 'master' into ankn/rc-no-exposure

a9eb1d2

Ank4n marked this pull request as ready for review March 15, 2025 19:30

Ank4n requested review from acatangiu and a team as code owners March 15, 2025 19:30

Ank4n removed request for a team and acatangiu March 15, 2025 19:31

paritytech-review-bot bot requested a review from a team March 15, 2025 19:32

github-actions bot and others added 2 commits March 15, 2025 19:32

Update from github-actions[bot] running command 'prdoc --force'

74220bd

fix session manager impl for ExistenceOf

6f0f00a

Ank4n and others added 2 commits March 15, 2025 23:37

if validator, return exists

a73ce62

Merge branch 'master' into ankn/rc-no-exposure

97ec539

kianenigma reviewed Mar 16, 2025

View reviewed changes

Ank4n commented Mar 16, 2025

View reviewed changes

kianenigma reviewed Mar 16, 2025

View reviewed changes

kianenigma previously approved these changes Mar 16, 2025

View reviewed changes

kianenigma reviewed Mar 16, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AHM] Replace Validator FullIdentification from `Exposure` to `Existence` #7936

[AHM] Replace Validator FullIdentification from `Exposure` to `Existence` #7936

Ank4n commented Mar 15, 2025 •

edited

Loading

Ank4n commented Mar 15, 2025

paritytech-workflow-stopper bot commented Mar 15, 2025

kianenigma Mar 16, 2025 •

edited

Loading

Ank4n Mar 16, 2025

kianenigma Mar 16, 2025

Ank4n Mar 16, 2025

kianenigma Mar 16, 2025

Ank4n Mar 16, 2025

kianenigma left a comment

kianenigma Mar 16, 2025

kianenigma Mar 16, 2025

kianenigma Mar 16, 2025

kianenigma Mar 16, 2025

iulianbarbu commented Mar 16, 2025

[AHM] Replace Validator FullIdentification from Exposure to Existence #7936

Are you sure you want to change the base?

[AHM] Replace Validator FullIdentification from Exposure to Existence #7936

Conversation

Ank4n commented Mar 15, 2025 • edited Loading

TODO

Ank4n commented Mar 15, 2025

paritytech-workflow-stopper bot commented Mar 15, 2025

kianenigma Mar 16, 2025 • edited Loading

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

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

iulianbarbu commented Mar 16, 2025

[AHM] Replace Validator FullIdentification from `Exposure` to `Existence` #7936

[AHM] Replace Validator FullIdentification from `Exposure` to `Existence` #7936

Ank4n commented Mar 15, 2025 •

edited

Loading

kianenigma Mar 16, 2025 •

edited

Loading