pallet-grandpa: Remove GRANDPA_AUTHORITIES_KEY (#2181)

Remove the `GRANDPA_AUTHORITIES_KEY` key and its usage. Apparently this
was used in the early days to communicate the grandpa authorities to the
node. However, we have now a runtime api that does this for us. So, this
pull request is moving from the custom managed storage item to a FRAME
managed storage item.

This pr also includes a migration for doing the switch on a running
chain.

---------

Co-authored-by: Davide Galassi <davxy@datawok.net>

polkadot/runtime/rococo/src/lib.rs

@@ -1497,6 +1497,8 @@ pub mod migrations {
 		frame_support::migrations::RemovePallet<PhragmenElectionPalletName, <Runtime as frame_system::Config>::DbWeight>,
 		frame_support::migrations::RemovePallet<TechnicalMembershipPalletName, <Runtime as frame_system::Config>::DbWeight>,
 		frame_support::migrations::RemovePallet<TipsPalletName, <Runtime as frame_system::Config>::DbWeight>,
+
+		pallet_grandpa::migrations::MigrateV4ToV5<Runtime>,
 	);
 }
 

polkadot/runtime/westend/src/lib.rs

@@ -1558,6 +1558,7 @@ pub mod migrations {
 		pallet_nomination_pools::migration::versioned_migrations::V5toV6<Runtime>,
 		pallet_referenda::migration::v1::MigrateV0ToV1<Runtime, ()>,
 		pallet_nomination_pools::migration::versioned_migrations::V6ToV7<Runtime>,
+		pallet_grandpa::migrations::MigrateV4ToV5<Runtime>,
 	);
 }
 

substrate/client/consensus/grandpa/src/lib.rs

@@ -471,9 +471,6 @@ where
 	Client: ExecutorProvider<Block, Executor = E> + HeaderBackend<Block>,
 {
 	fn get(&self) -> Result<AuthorityList, ClientError> {
-		// This implementation uses the Grandpa runtime API instead of reading directly from the
-		// `GRANDPA_AUTHORITIES_KEY` as the data may have been migrated since the genesis block of
-		// the chain, whereas the runtime API is backwards compatible.
 		self.executor()
 			.call(
 				self.expect_block_hash_from_id(&BlockId::Number(Zero::zero()))?,

substrate/frame/grandpa/src/lib.rs

@@ -30,14 +30,13 @@
 
 // Re-export since this is necessary for `impl_apis` in runtime.
 pub use sp_consensus_grandpa::{
-	self as fg_primitives, AuthorityId, AuthorityList, AuthorityWeight, VersionedAuthorityList,
+	self as fg_primitives, AuthorityId, AuthorityList, AuthorityWeight,
 };
 
-use codec::{self as codec, Decode, Encode, MaxEncodedLen};
+use codec::{Decode, Encode, MaxEncodedLen};
 use frame_support::{
 	dispatch::{DispatchResultWithPostInfo, Pays},
 	pallet_prelude::Get,
-	storage,
 	traits::OneSessionHandler,
 	weights::Weight,
 	WeakBoundedVec,
@@ -45,8 +44,8 @@ use frame_support::{
 use frame_system::pallet_prelude::BlockNumberFor;
 use scale_info::TypeInfo;
 use sp_consensus_grandpa::{
-	ConsensusLog, EquivocationProof, ScheduledChange, SetId, GRANDPA_AUTHORITIES_KEY,
+	ConsensusLog, EquivocationProof, ScheduledChange, SetId, GRANDPA_ENGINE_ID,
-	GRANDPA_ENGINE_ID, RUNTIME_LOG_TARGET as LOG_TARGET,
+	RUNTIME_LOG_TARGET as LOG_TARGET,
 };
 use sp_runtime::{generic::DigestItem, traits::Zero, DispatchResult};
 use sp_session::{GetSessionNumber, GetValidatorCount};
@@ -75,7 +74,7 @@ pub mod pallet {
 	use frame_system::pallet_prelude::*;
 
 	/// The current storage version.
-	const STORAGE_VERSION: StorageVersion = StorageVersion::new(4);
+	const STORAGE_VERSION: StorageVersion = StorageVersion::new(5);
 
 	#[pallet::pallet]
 	#[pallet::storage_version(STORAGE_VERSION)]
@@ -145,7 +144,7 @@ pub mod pallet {
 
 				// enact the change if we've reached the enacting block
 				if block_number == pending_change.scheduled_at + pending_change.delay {
-					Self::set_grandpa_authorities(&pending_change.next_authorities);
+					Authorities::<T>::put(&pending_change.next_authorities);
 					Self::deposit_event(Event::NewAuthorities {
 						authority_set: pending_change.next_authorities.into_inner(),
 					});
@@ -342,6 +341,11 @@ pub mod pallet {
 	#[pallet::getter(fn session_for_set)]
 	pub(super) type SetIdSession<T: Config> = StorageMap<_, Twox64Concat, SetId, SessionIndex>;
 
+	/// The current list of authorities.
+	#[pallet::storage]
+	pub(crate) type Authorities<T: Config> =
+		StorageValue<_, BoundedAuthorityList<T::MaxAuthorities>, ValueQuery>;
+
 	#[derive(frame_support::DefaultNoBound)]
 	#[pallet::genesis_config]
 	pub struct GenesisConfig<T: Config> {
@@ -354,7 +358,7 @@ pub mod pallet {
 	impl<T: Config> BuildGenesisConfig for GenesisConfig<T> {
 		fn build(&self) {
 			CurrentSetId::<T>::put(SetId::default());
-			Pallet::<T>::initialize(&self.authorities)
+			Pallet::<T>::initialize(self.authorities.clone())
 		}
 	}
 
@@ -428,12 +432,7 @@ pub enum StoredState<N> {
 impl<T: Config> Pallet<T> {
 	/// Get the current set of authorities, along with their respective weights.
 	pub fn grandpa_authorities() -> AuthorityList {
-		storage::unhashed::get_or_default::<VersionedAuthorityList>(GRANDPA_AUTHORITIES_KEY).into()
+		Authorities::<T>::get().into_inner()
-	}
-
-	/// Set the current set of authorities, along with their respective weights.
-	fn set_grandpa_authorities(authorities: &AuthorityList) {
-		storage::unhashed::put(GRANDPA_AUTHORITIES_KEY, &VersionedAuthorityList::from(authorities));
 	}
 
 	/// Schedule GRANDPA to pause starting in the given number of blocks.
@@ -522,10 +521,14 @@ impl<T: Config> Pallet<T> {
 
 	// Perform module initialization, abstracted so that it can be called either through genesis
 	// config builder or through `on_genesis_session`.
-	fn initialize(authorities: &AuthorityList) {
+	fn initialize(authorities: AuthorityList) {
 		if !authorities.is_empty() {
 			assert!(Self::grandpa_authorities().is_empty(), "Authorities are already initialized!");
-			Self::set_grandpa_authorities(authorities);
+			Authorities::<T>::put(
+				&BoundedAuthorityList::<T::MaxAuthorities>::try_from(authorities).expect(
+					"Grandpa: `Config::MaxAuthorities` is smaller than the number of genesis authorities!",
+				),
+			);
 		}
 
 		// NOTE: initialize first session of first set. this is necessary for
@@ -568,7 +571,7 @@ where
 		I: Iterator<Item = (&'a T::AccountId, AuthorityId)>,
 	{
 		let authorities = validators.map(|(_, k)| (k, 1)).collect::<Vec<_>>();
-		Self::initialize(&authorities);
+		Self::initialize(authorities);
 	}
 
 	fn on_new_session<'a, I: 'a>(changed: bool, validators: I, _queued_validators: I)

substrate/frame/grandpa/src/migrations.rs

@@ -22,8 +22,11 @@ use frame_support::{
 
 use crate::{Config, CurrentSetId, SetIdSession, LOG_TARGET};
 
+pub use v5::MigrateV4ToV5;
+
 /// Version 4.
 pub mod v4;
+mod v5;
 
 /// This migration will clean up all stale set id -> session entries from the
 /// `SetIdSession` storage map, only the latest `max_set_id_session_entries`

substrate/frame/grandpa/src/migrations/v5.rs

@@ -0,0 +1,96 @@
+// This file is part of Substrate.
+
+// Copyright (C) Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use crate::{BoundedAuthorityList, Pallet};
+use codec::Decode;
+use frame_support::{
+	migrations::VersionedMigration,
+	storage,
+	traits::{Get, OnRuntimeUpgrade},
+	weights::Weight,
+};
+use sp_consensus_grandpa::AuthorityList;
+use sp_std::{marker::PhantomData, vec::Vec};
+
+const GRANDPA_AUTHORITIES_KEY: &[u8] = b":grandpa_authorities";
+
+fn load_authority_list() -> AuthorityList {
+	storage::unhashed::get_raw(GRANDPA_AUTHORITIES_KEY).map_or_else(
+		|| Vec::new(),
+		|l| <(u8, AuthorityList)>::decode(&mut &l[..]).unwrap_or_default().1,
+	)
+}
+
+/// Actual implementation of [`MigrateV4ToV5`].
+pub struct MigrateImpl<T>(PhantomData<T>);
+
+impl<T: crate::Config> OnRuntimeUpgrade for MigrateImpl<T> {
+	#[cfg(feature = "try-runtime")]
+	fn pre_upgrade() -> Result<Vec<u8>, sp_runtime::TryRuntimeError> {
+		use codec::Encode;
+
+		let authority_list_len = load_authority_list().len() as u32;
+
+		if authority_list_len > T::MaxAuthorities::get() {
+			return Err(
+				"Grandpa: `Config::MaxAuthorities` is smaller than the actual number of authorities.".into()
+			)
+		}
+
+		if authority_list_len == 0 {
+			return Err("Grandpa: Authority list is empty!".into())
+		}
+
+		Ok(authority_list_len.encode())
+	}
+
+	#[cfg(feature = "try-runtime")]
+	fn post_upgrade(state: Vec<u8>) -> Result<(), sp_runtime::TryRuntimeError> {
+		let len = u32::decode(&mut &state[..]).unwrap();
+
+		frame_support::ensure!(
+			len == crate::Pallet::<T>::grandpa_authorities().len() as u32,
+			"Grandpa: pre-migrated and post-migrated list should have the same length"
+		);
+
+		frame_support::ensure!(
+			load_authority_list().is_empty(),
+			"Old authority list shouldn't exist anymore"
+		);
+
+		Ok(())
+	}
+
+	fn on_runtime_upgrade() -> Weight {
+		crate::Authorities::<T>::put(
+			&BoundedAuthorityList::<T::MaxAuthorities>::force_from(
+				load_authority_list(),
+				Some("Grandpa: `Config::MaxAuthorities` is smaller than the actual number of authorities.")
+			)
+		);
+
+		storage::unhashed::kill(GRANDPA_AUTHORITIES_KEY);
+
+		T::DbWeight::get().reads_writes(1, 2)
+	}
+}
+
+/// Migrate the storage from V4 to V5.
+///
+/// Switches from `GRANDPA_AUTHORITIES_KEY` to a normal FRAME storage item.
+pub type MigrateV4ToV5<T> =
+	VersionedMigration<4, 5, MigrateImpl<T>, Pallet<T>, <T as frame_system::Config>::DbWeight>;

substrate/primitives/consensus/grandpa/src/lib.rs

@@ -19,13 +19,10 @@
 
 #![cfg_attr(not(feature = "std"), no_std)]
 
-#[cfg(not(feature = "std"))]
-extern crate alloc;
-
 #[cfg(feature = "serde")]
 use serde::Serialize;
 
-use codec::{Codec, Decode, Encode, Input};
+use codec::{Codec, Decode, Encode};
 use scale_info::TypeInfo;
 #[cfg(feature = "std")]
 use sp_keystore::KeystorePtr;
@@ -33,7 +30,7 @@ use sp_runtime::{
 	traits::{Header as HeaderT, NumberFor},
 	ConsensusEngineId, RuntimeDebug,
 };
-use sp_std::{borrow::Cow, vec::Vec};
+use sp_std::vec::Vec;
 
 /// The log target to be used by client code.
 pub const CLIENT_LOG_TARGET: &str = "grandpa";
@@ -62,10 +59,6 @@ pub type AuthoritySignature = app::Signature;
 /// The `ConsensusEngineId` of GRANDPA.
 pub const GRANDPA_ENGINE_ID: ConsensusEngineId = *b"FRNK";
 
-/// The storage key for the current set of weighted Grandpa authorities.
-/// The value stored is an encoded VersionedAuthorityList.
-pub const GRANDPA_AUTHORITIES_KEY: &[u8] = b":grandpa_authorities";
-
 /// The weight of an authority.
 pub type AuthorityWeight = u64;
 
@@ -464,60 +457,6 @@ where
 	Some(grandpa::SignedMessage { message, signature, id: public })
 }
 
-/// WASM function call to check for pending changes.
-pub const PENDING_CHANGE_CALL: &str = "grandpa_pending_change";
-/// WASM function call to get current GRANDPA authorities.
-pub const AUTHORITIES_CALL: &str = "grandpa_authorities";
-
-/// The current version of the stored AuthorityList type. The encoding version MUST be updated any
-/// time the AuthorityList type changes.
-const AUTHORITIES_VERSION: u8 = 1;
-
-/// An AuthorityList that is encoded with a version specifier. The encoding version is updated any
-/// time the AuthorityList type changes. This ensures that encodings of different versions of an
-/// AuthorityList are differentiable. Attempting to decode an authority list with an unknown
-/// version will fail.
-#[derive(Default)]
-pub struct VersionedAuthorityList<'a>(Cow<'a, AuthorityList>);
-
-impl<'a> From<AuthorityList> for VersionedAuthorityList<'a> {
-	fn from(authorities: AuthorityList) -> Self {
-		VersionedAuthorityList(Cow::Owned(authorities))
-	}
-}
-
-impl<'a> From<&'a AuthorityList> for VersionedAuthorityList<'a> {
-	fn from(authorities: &'a AuthorityList) -> Self {
-		VersionedAuthorityList(Cow::Borrowed(authorities))
-	}
-}
-
-impl<'a> Into<AuthorityList> for VersionedAuthorityList<'a> {
-	fn into(self) -> AuthorityList {
-		self.0.into_owned()
-	}
-}
-
-impl<'a> Encode for VersionedAuthorityList<'a> {
-	fn size_hint(&self) -> usize {
-		(AUTHORITIES_VERSION, self.0.as_ref()).size_hint()
-	}
-
-	fn using_encoded<R, F: FnOnce(&[u8]) -> R>(&self, f: F) -> R {
-		(AUTHORITIES_VERSION, self.0.as_ref()).using_encoded(f)
-	}
-}
-
-impl<'a> Decode for VersionedAuthorityList<'a> {
-	fn decode<I: Input>(value: &mut I) -> Result<Self, codec::Error> {
-		let (version, authorities): (u8, AuthorityList) = Decode::decode(value)?;
-		if version != AUTHORITIES_VERSION {
-			return Err("unknown Grandpa authorities version".into())
-		}
-		Ok(authorities.into())
-	}
-}
-
 /// An opaque type used to represent the key ownership proof at the runtime API
 /// boundary. The inner value is an encoded representation of the actual key
 /// ownership proof which will be parameterized when defining the runtime. At