Try State Hook for Beefy (#3246)

Part of: #239

Polkadot address: 12GyGD3QhT4i2JJpNzvMf96sxxBLWymz4RdGCxRH5Rj5agKW

prdoc/pr_3246.prdoc

@@ -0,0 +1,11 @@
+title: Try State Hook for Beefy.
+
+doc:
+  - audience: Runtime User
+    description: |
+      Invariants for storage items in the beefy pallet. Enforces the following Invariants:
+      1. `Authorities` should not exceed the `MaxAuthorities` capacity.
+      2. `NextAuthorities` should not exceed the `MaxAuthorities` capacity.
+      3. `ValidatorSetId` must be present in `SetIdSession`.
+crates:
+- name: pallet-beefy

substrate/frame/beefy/src/lib.rs

@@ -280,6 +280,14 @@ pub mod pallet {
 		}
 	}
 
+	#[pallet::hooks]
+	impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
+		#[cfg(feature = "try-runtime")]
+		fn try_state(_n: BlockNumberFor<T>) -> Result<(), sp_runtime::TryRuntimeError> {
+			Self::do_try_state()
+		}
+	}
+
 	#[pallet::validate_unsigned]
 	impl<T: Config> ValidateUnsigned for Pallet<T> {
 		type Call = Call<T>;
@@ -294,6 +302,60 @@ pub mod pallet {
 	}
 }
 
+#[cfg(any(feature = "try-runtime", test))]
+impl<T: Config> Pallet<T> {
+	/// Ensure the correctness of the state of this pallet.
+	///
+	/// This should be valid before or after each state transition of this pallet.
+	pub fn do_try_state() -> Result<(), sp_runtime::TryRuntimeError> {
+		Self::try_state_authorities()?;
+		Self::try_state_validators()?;
+
+		Ok(())
+	}
+
+	/// # Invariants
+	///
+	/// * `Authorities` should not exceed the `MaxAuthorities` capacity.
+	/// * `NextAuthorities` should not exceed the `MaxAuthorities` capacity.
+	fn try_state_authorities() -> Result<(), sp_runtime::TryRuntimeError> {
+		if let Some(authorities_len) = <Authorities<T>>::decode_len() {
+			ensure!(
+				authorities_len as u32 <= T::MaxAuthorities::get(),
+				"Authorities number exceeds what the pallet config allows."
+			);
+		} else {
+			return Err(sp_runtime::TryRuntimeError::Other(
+				"Failed to decode length of authorities",
+			));
+		}
+
+		if let Some(next_authorities_len) = <NextAuthorities<T>>::decode_len() {
+			ensure!(
+				next_authorities_len as u32 <= T::MaxAuthorities::get(),
+				"Next authorities number exceeds what the pallet config allows."
+			);
+		} else {
+			return Err(sp_runtime::TryRuntimeError::Other(
+				"Failed to decode length of next authorities",
+			));
+		}
+		Ok(())
+	}
+
+	/// # Invariants
+	///
+	/// `ValidatorSetId` must be present in `SetIdSession`
+	fn try_state_validators() -> Result<(), sp_runtime::TryRuntimeError> {
+		let validator_set_id = <ValidatorSetId<T>>::get();
+		ensure!(
+			SetIdSession::<T>::get(validator_set_id).is_some(),
+			"Validator set id must be present in SetIdSession"
+		);
+		Ok(())
+	}
+}
+
 impl<T: Config> Pallet<T> {
 	/// Return the current active BEEFY validator set.
 	pub fn validator_set() -> Option<ValidatorSet<T::BeefyId>> {

substrate/frame/beefy/src/mock.rs

@@ -27,7 +27,6 @@ use frame_support::{
 };
 use pallet_session::historical as pallet_session_historical;
 use sp_core::{crypto::KeyTypeId, ConstU128};
-use sp_io::TestExternalities;
 use sp_runtime::{
 	app_crypto::ecdsa::Public, curve::PiecewiseLinear, impl_opaque_keys, testing::TestXt,
 	traits::OpaqueKeys, BuildStorage, Perbill,
@@ -210,6 +209,73 @@ impl pallet_offences::Config for Test {
 	type OnOffenceHandler = Staking;
 }
 
+#[derive(Default)]
+pub struct ExtBuilder {
+	authorities: Vec<BeefyId>,
+}
+
+impl ExtBuilder {
+	/// Add some AccountIds to insert into `List`.
+	#[cfg(test)]
+	pub(crate) fn add_authorities(mut self, ids: Vec<BeefyId>) -> Self {
+		self.authorities = ids;
+		self
+	}
+
+	pub fn build(self) -> sp_io::TestExternalities {
+		let mut t = frame_system::GenesisConfig::<Test>::default().build_storage().unwrap();
+
+		let balances: Vec<_> =
+			(0..self.authorities.len()).map(|i| (i as u64, 10_000_000)).collect();
+
+		pallet_balances::GenesisConfig::<Test> { balances }
+			.assimilate_storage(&mut t)
+			.unwrap();
+
+		let session_keys: Vec<_> = self
+			.authorities
+			.iter()
+			.enumerate()
+			.map(|(i, k)| (i as u64, i as u64, MockSessionKeys { dummy: k.clone() }))
+			.collect();
+
+		BasicExternalities::execute_with_storage(&mut t, || {
+			for (ref id, ..) in &session_keys {
+				frame_system::Pallet::<Test>::inc_providers(id);
+			}
+		});
+
+		pallet_session::GenesisConfig::<Test> { keys: session_keys }
+			.assimilate_storage(&mut t)
+			.unwrap();
+
+		// controllers are same as stash
+		let stakers: Vec<_> = (0..self.authorities.len())
+			.map(|i| (i as u64, i as u64, 10_000, pallet_staking::StakerStatus::<u64>::Validator))
+			.collect();
+
+		let staking_config = pallet_staking::GenesisConfig::<Test> {
+			stakers,
+			validator_count: 2,
+			force_era: pallet_staking::Forcing::ForceNew,
+			minimum_validator_count: 0,
+			invulnerables: vec![],
+			..Default::default()
+		};
+
+		staking_config.assimilate_storage(&mut t).unwrap();
+
+		t.into()
+	}
+
+	pub fn build_and_execute(self, test: impl FnOnce() -> ()) {
+		self.build().execute_with(|| {
+			test();
+			Beefy::do_try_state().expect("All invariants must hold after a test");
+		})
+	}
+}
+
 // Note, that we can't use `UintAuthorityId` here. Reason is that the implementation
 // of `to_public_key()` assumes, that a public key is 32 bytes long. This is true for
 // ed25519 and sr25519 but *not* for ecdsa. A compressed ecdsa public key is 33 bytes,
@@ -226,54 +292,6 @@ pub fn mock_authorities(vec: Vec<u8>) -> Vec<BeefyId> {
 	vec.into_iter().map(|id| mock_beefy_id(id)).collect()
 }
 
-pub fn new_test_ext(ids: Vec<u8>) -> TestExternalities {
-	new_test_ext_raw_authorities(mock_authorities(ids))
-}
-
-pub fn new_test_ext_raw_authorities(authorities: Vec<BeefyId>) -> TestExternalities {
-	let mut t = frame_system::GenesisConfig::<Test>::default().build_storage().unwrap();
-
-	let balances: Vec<_> = (0..authorities.len()).map(|i| (i as u64, 10_000_000)).collect();
-
-	pallet_balances::GenesisConfig::<Test> { balances }
-		.assimilate_storage(&mut t)
-		.unwrap();
-
-	let session_keys: Vec<_> = authorities
-		.iter()
-		.enumerate()
-		.map(|(i, k)| (i as u64, i as u64, MockSessionKeys { dummy: k.clone() }))
-		.collect();
-
-	BasicExternalities::execute_with_storage(&mut t, || {
-		for (ref id, ..) in &session_keys {
-			frame_system::Pallet::<Test>::inc_providers(id);
-		}
-	});
-
-	pallet_session::GenesisConfig::<Test> { keys: session_keys }
-		.assimilate_storage(&mut t)
-		.unwrap();
-
-	// controllers are same as stash
-	let stakers: Vec<_> = (0..authorities.len())
-		.map(|i| (i as u64, i as u64, 10_000, pallet_staking::StakerStatus::<u64>::Validator))
-		.collect();
-
-	let staking_config = pallet_staking::GenesisConfig::<Test> {
-		stakers,
-		validator_count: 2,
-		force_era: pallet_staking::Forcing::ForceNew,
-		minimum_validator_count: 0,
-		invulnerables: vec![],
-		..Default::default()
-	};
-
-	staking_config.assimilate_storage(&mut t).unwrap();
-
-	t.into()
-}
-
 pub fn start_session(session_index: SessionIndex) {
 	for i in Session::current_index()..session_index {
 		System::on_finalize(System::block_number());