BABE/GRANDPA equivocations are always reported for the latest possible session

[kayabaNerve]

serai/substrate/runtime/src/wasm/babe_grandpa/babe.rs

Lines 340 to 357 in e962cf4

	// Find the most recent (non-historical) session we can associate this with
	let mut outer_session = ValidatorSets::current_session(NetworkId::Serai);
	let mut completed_iters = 0;
	while let Some(inner_session) = outer_session {
	let Some(validator) =
	ValidatorSets::selected_validators_with_serai_auxiliary_keys(ValidatorSet {
	network: NetworkId::Serai,
	session: inner_session,
	})
	.find_map(|(validator, aux_key)| {
	(serai_validator_sets_pallet::subkey(&aux_key, key_types::BABE) == subkey)
	.then_some(validator)
	})
	else {
	completed_iters += 1;
	outer_session = inner_session.0.checked_sub(1).map(Session).filter(|_| completed_iters < 3);
	continue;
	};

serai/substrate/runtime/src/wasm/babe_grandpa/grandpa.rs

Lines 41 to 58 in e962cf4

	// Find the most recent (non-historical) session we can associate this with
	let mut outer_session = ValidatorSets::current_session(NetworkId::Serai);
	let mut completed_iters = 0;
	while let Some(inner_session) = outer_session {
	let Some(validator) =
	ValidatorSets::selected_validators_with_serai_auxiliary_keys(ValidatorSet {
	network: NetworkId::Serai,
	session: inner_session,
	})
	.find_map(|(validator, aux_key)| {
	(serai_validator_sets_pallet::subkey(&aux_key, key_types::GRANDPA) == subkey)
	.then_some(validator)
	})
	else {
	completed_iters += 1;
	outer_session = inner_session.0.checked_sub(1).map(Session).filter(|_| completed_iters < 3);
	continue;
	};

This was originally written as the session doesn't effect how the slash, regarding allocations, is performed. The fatal_slash method does not input the session, and instead always slashes as possible based on the current state: https://github.com/serai-dex/serai/blob/next-polkadot-sdk/substrate/validator-sets/src/sessions/slash_reports.rs#L79-L93

The issue is two-fold:

1. This session is used in an event, so this will have inaccurate logging regarding it:

   serai/substrate/validator-sets/src/sessions/slash_reports.rs

   Lines 255 to 259 in e962cf4

   	fn slash_serai_validator(session: Session, validator: SeraiAddress) {
   	let network = NetworkId::Serai;
   	let set = ValidatorSet { network, session };
   	Core::<Storage::Config>::emit_event(Event::Slashes { set });
   	fatal_slash::<Self>(network, validator);

This may be fine to enshrine as the expected behavior however. Misbehavior during any session being able to be used to slash the validator in the current session would be fine.

2. Whether or not we emit the on_disabled consensus log is dependent on session == current_session, despite DisabledValidators always considering a successfully fatally slashed validator as disabled:

serai/substrate/validator-sets/src/sessions/slash_reports.rs

Lines 261 to 273 in e962cf4

	// If this for the current session, disable the validator
	if session ==
	crate::pallet::CurrentSession::<Storage::Config>::get(network)
	.expect("slashing Serai validator yet no current session for Serai?")
	{
	// Panicking here is fine as a majority of Serai validators approved an invalid inherent
	let i = crate::Pallet::<Storage::Config>::selected_validators(set)
	.position(|(this_validator, _)| this_validator == validator)
	.expect("slashing Serai validator who was not in the alleged session");
	let i = u32::try_from(i).unwrap();
	crate::Babe::<Storage::Config>::on_disabled(i);
	crate::Grandpa::<Storage::Config>::on_disabled(i);
	}

https://github.com/serai-dex/serai/blob/e962cf49c8bb61f5779f10ad342cd075457bfd1b/substrate/validator-sets/src/sessions/slash_reports.rs#L881-L896

This is further incongruent in that we may emit the on_disabled consensus log for genesis validators despite how we'll refuse to disable genesis validators, believing them trusted entities.

The solution would appear two-fold:

1. Drop the session field from the Slashes::Serai variant, enshrining that we consider any misbehavior during any session to permanently invalidate the validator:

   serai/substrate/abi/src/modules/validator_sets.rs

   Lines 20 to 26 in e962cf4

   	/// Slash(es) to occur on-chain.
   	#[derive(Clone, PartialEq, Eq, Debug)]
   	pub enum Slashes {
   	/// A single slash for a specific validator of the Serai network.
   	Serai {
   	/// The session the misbehavior occurred during.
   	session: Session,

2. Replace the if conditional of session == current_session, determining if we emit on_disabled or not, with DisabledValidators::is_disabled. This would ensure the logic for if we just disabled the current validator is equivalent to the logic for if a validator is disabled in general, where thanks to our validate_unsigned filtering out already-applied slashes, we can assume that any fatally-slashed Serai validator who now has is_disabled(_) == true has only just become disabled.

---

As-is, this is solely an oddity with the logs and mishandling of on_disabled, where on_disabled produces ConsensusLog::OnDisabled events (BABE, GRANDPA). The Serai node does not use these events, nor does the Serai ABI expose them in any way. Their sole purpose, presumably, would be for light clients to learn a validator was disabled and adjust accordingly, but within the Serai flow, the inclusion of a report_slashes transaction should be used to prove that a slash was reported. Accordingly, one could take the liberty of handwaving this to solely the oddity with the logs, though that would be quite an aggressive opinion.

[kayabaNerve]

This should be able to be closed, but I'll defer doing so until after the current audit resolves.