Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pallet-grandpa: Remove GRANDPA_AUTHORITIES_KEY #2181

Merged
merged 10 commits into from
Nov 13, 2023
2 changes: 2 additions & 0 deletions polkadot/runtime/rococo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
);
}

Expand Down
1 change: 1 addition & 0 deletions polkadot/runtime/westend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
);
}

Expand Down
3 changes: 0 additions & 3 deletions substrate/client/consensus/grandpa/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()))?,
Expand Down
37 changes: 20 additions & 17 deletions substrate/frame/grandpa/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,23 +30,22 @@

// 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,
};
use frame_system::pallet_prelude::BlockNumberFor;
use scale_info::TypeInfo;
use sp_consensus_grandpa::{
ConsensusLog, EquivocationProof, ScheduledChange, SetId, GRANDPA_AUTHORITIES_KEY,
GRANDPA_ENGINE_ID, RUNTIME_LOG_TARGET as LOG_TARGET,
ConsensusLog, EquivocationProof, ScheduledChange, SetId, GRANDPA_ENGINE_ID,
RUNTIME_LOG_TARGET as LOG_TARGET,
};
use sp_runtime::{generic::DigestItem, traits::Zero, DispatchResult};
use sp_session::{GetSessionNumber, GetValidatorCount};
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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(),
});
Expand Down Expand Up @@ -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> {
Expand All @@ -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())
}
}

Expand Down Expand Up @@ -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()
}

/// 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));
Authorities::<T>::get().into_inner()
}

/// Schedule GRANDPA to pause starting in the given number of blocks.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions substrate/frame/grandpa/src/migrations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
96 changes: 96 additions & 0 deletions substrate/frame/grandpa/src/migrations/v5.rs
Original file line number Diff line number Diff line change
@@ -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() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this fail as well if authority_list_len is 0?

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>;
65 changes: 2 additions & 63 deletions substrate/primitives/consensus/grandpa/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,18 @@

#![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;
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";
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down