From b50596428eefac2afe6c08809d5a33632c88522c Mon Sep 17 00:00:00 2001
From: Robert Habermeier <rphmeier@gmail.com>
Date: Sat, 24 Aug 2019 17:17:01 +0200
Subject: [PATCH] GRANDPA links set IDs to sessions. (#3472)
* introduce some type aliases for round and set-id
* overhaul session "changed" flag and document better
* do_initialize in BABE when getting new session
* grandpa module tracks set IDs
* update runtime versions
* doc comment -> comment
* Include docs fixes from Gav
Co-Authored-By: Gavin Wood <gavin@parity.io>
* some more review changes
* fix srml-grandpa compilation
---
.../finality-grandpa/primitives/src/lib.rs | 6 +
.../core/finality-grandpa/src/aux_schema.rs | 20 ++--
.../finality-grandpa/src/communication/mod.rs | 36 +++---
.../core/finality-grandpa/src/environment.rs | 50 +++++----
substrate/core/finality-grandpa/src/lib.rs | 8 +-
.../core/finality-grandpa/src/light_import.rs | 4 +-
substrate/node/runtime/src/lib.rs | 2 +-
substrate/srml/babe/src/lib.rs | 2 +
substrate/srml/grandpa/src/lib.rs | 64 +++++++----
substrate/srml/session/src/lib.rs | 103 +++++++++++++-----
substrate/srml/session/src/mock.rs | 29 +++--
substrate/srml/staking/src/lib.rs | 4 +
12 files changed, 214 insertions(+), 114 deletions(-)
diff --git a/substrate/core/finality-grandpa/primitives/src/lib.rs b/substrate/core/finality-grandpa/primitives/src/lib.rs
index b92444e2629..1f103a548d1 100644
--- a/substrate/core/finality-grandpa/primitives/src/lib.rs
+++ b/substrate/core/finality-grandpa/primitives/src/lib.rs
@@ -52,6 +52,12 @@ pub type AuthorityWeight = u64;
/// The index of an authority.
pub type AuthorityIndex = u64;
+/// The identifier of a GRANDPA set.
+pub type SetId = u64;
+
+/// The round indicator.
+pub type RoundNumber = u64;
+
/// A scheduled change of authority set.
#[cfg_attr(feature = "std", derive(Debug, Serialize))]
#[derive(Clone, Eq, PartialEq, Encode, Decode)]
diff --git a/substrate/core/finality-grandpa/src/aux_schema.rs b/substrate/core/finality-grandpa/src/aux_schema.rs
index 78c1741d519..599604c1d32 100644
--- a/substrate/core/finality-grandpa/src/aux_schema.rs
+++ b/substrate/core/finality-grandpa/src/aux_schema.rs
@@ -26,7 +26,7 @@ use grandpa::round::State as RoundState;
use sr_primitives::traits::{Block as BlockT, NumberFor};
use log::{info, warn};
use substrate_telemetry::{telemetry, CONSENSUS_INFO};
-use fg_primitives::AuthorityId;
+use fg_primitives::{AuthorityId, AuthorityWeight, SetId, RoundNumber};
use crate::authorities::{AuthoritySet, SharedAuthoritySet, PendingChange, DelayKind};
use crate::consensus_changes::{SharedConsensusChanges, ConsensusChanges};
@@ -47,16 +47,16 @@ const CURRENT_VERSION: u32 = 2;
#[cfg_attr(test, derive(PartialEq))]
pub enum V1VoterSetState<H, N> {
/// The voter set state, currently paused.
- Paused(u64, RoundState<H, N>),
+ Paused(RoundNumber, RoundState<H, N>),
/// The voter set state, currently live.
- Live(u64, RoundState<H, N>),
+ Live(RoundNumber, RoundState<H, N>),
}
-type V0VoterSetState<H, N> = (u64, RoundState<H, N>);
+type V0VoterSetState<H, N> = (RoundNumber, RoundState<H, N>);
#[derive(Debug, Clone, Encode, Decode, PartialEq)]
struct V0PendingChange<H, N> {
- next_authorities: Vec<(AuthorityId, u64)>,
+ next_authorities: Vec<(AuthorityId, AuthorityWeight)>,
delay: N,
canon_height: N,
canon_hash: H,
@@ -64,8 +64,8 @@ struct V0PendingChange<H, N> {
#[derive(Debug, Clone, Encode, Decode, PartialEq)]
struct V0AuthoritySet<H, N> {
- current_authorities: Vec<(AuthorityId, u64)>,
- set_id: u64,
+ current_authorities: Vec<(AuthorityId, AuthorityWeight)>,
+ set_id: SetId,
pending_changes: Vec<V0PendingChange<H, N>>,
}
@@ -267,7 +267,7 @@ pub(crate) fn load_persistent<Block: BlockT, B, G>(
-> ClientResult<PersistentData<Block>>
where
B: AuxStore,
- G: FnOnce() -> ClientResult<Vec<(AuthorityId, u64)>>,
+ G: FnOnce() -> ClientResult<Vec<(AuthorityId, AuthorityWeight)>>,
{
let version: Option<u32> = load_decode(backend, VERSION_KEY)?;
let consensus_changes = load_decode(backend, CONSENSUS_CHANGES_KEY)?
@@ -448,7 +448,7 @@ mod test {
let authorities = vec![(AuthorityId::default(), 100)];
let set_id = 3;
- let round_number: u64 = 42;
+ let round_number: RoundNumber = 42;
let round_state = RoundState::<H256, u64> {
prevote_ghost: Some((H256::random(), 32)),
finalized: None,
@@ -536,7 +536,7 @@ mod test {
let authorities = vec![(AuthorityId::default(), 100)];
let set_id = 3;
- let round_number: u64 = 42;
+ let round_number: RoundNumber = 42;
let round_state = RoundState::<H256, u64> {
prevote_ghost: Some((H256::random(), 32)),
finalized: None,
diff --git a/substrate/core/finality-grandpa/src/communication/mod.rs b/substrate/core/finality-grandpa/src/communication/mod.rs
index a4b149fac46..732c14c1a9c 100644
--- a/substrate/core/finality-grandpa/src/communication/mod.rs
+++ b/substrate/core/finality-grandpa/src/communication/mod.rs
@@ -50,7 +50,9 @@ use crate::environment::HasVoted;
use gossip::{
GossipMessage, FullCatchUpMessage, FullCommitMessage, VoteOrPrecommitMessage, GossipValidator
};
-use fg_primitives::{AuthorityPair, AuthorityId, AuthoritySignature};
+use fg_primitives::{
+ AuthorityPair, AuthorityId, AuthoritySignature, SetId as SetIdNumber, RoundNumber,
+};
pub mod gossip;
mod periodic;
@@ -129,12 +131,12 @@ pub trait Network<Block: BlockT>: Clone + Send + 'static {
}
/// Create a unique topic for a round and set-id combo.
-pub(crate) fn round_topic<B: BlockT>(round: u64, set_id: u64) -> B::Hash {
+pub(crate) fn round_topic<B: BlockT>(round: RoundNumber, set_id: SetIdNumber) -> B::Hash {
<<B::Header as HeaderT>::Hashing as HashT>::hash(format!("{}-{}", set_id, round).as_bytes())
}
/// Create a unique topic for global messages on a set ID.
-pub(crate) fn global_topic<B: BlockT>(set_id: u64) -> B::Hash {
+pub(crate) fn global_topic<B: BlockT>(set_id: SetIdNumber) -> B::Hash {
<<B::Header as HeaderT>::Hashing as HashT>::hash(format!("{}-GLOBAL", set_id).as_bytes())
}
@@ -618,25 +620,25 @@ impl<B: BlockT, N: Network<B>> Clone for NetworkBridge<B, N> {
}
}
-fn localized_payload<E: Encode>(round: u64, set_id: u64, message: &E) -> Vec<u8> {
+fn localized_payload<E: Encode>(round: RoundNumber, set_id: SetIdNumber, message: &E) -> Vec<u8> {
(message, round, set_id).encode()
}
-/// Type-safe wrapper around u64 when indicating that it's a round number.
+/// Type-safe wrapper around a round number.
#[derive(Debug, Clone, Copy, Eq, PartialEq, PartialOrd, Ord, Encode, Decode)]
-pub struct Round(pub u64);
+pub struct Round(pub RoundNumber);
-/// Type-safe wrapper around u64 when indicating that it's a set ID.
+/// Type-safe wrapper around a set ID.
#[derive(Debug, Clone, Copy, Eq, PartialEq, PartialOrd, Ord, Encode, Decode)]
-pub struct SetId(pub u64);
+pub struct SetId(pub SetIdNumber);
// check a message.
pub(crate) fn check_message_sig<Block: BlockT>(
message: &Message<Block>,
id: &AuthorityId,
signature: &AuthoritySignature,
- round: u64,
- set_id: u64,
+ round: RoundNumber,
+ set_id: SetIdNumber,
) -> Result<(), ()> {
let as_public = id.clone();
let encoded_raw = localized_payload(round, set_id, message);
@@ -656,8 +658,8 @@ pub(crate) fn check_message_sig<Block: BlockT>(
/// `ed25519` and `BLS` signatures (which we might use in the future), care must
/// be taken when switching to different key types.
struct OutgoingMessages<Block: BlockT, N: Network<Block>> {
- round: u64,
- set_id: u64,
+ round: RoundNumber,
+ set_id: SetIdNumber,
locals: Option<(AuthorityPair, AuthorityId)>,
sender: mpsc::UnboundedSender<SignedMessage<Block>>,
network: N,
@@ -851,8 +853,8 @@ fn check_catch_up<Block: BlockT>(
fn check_signatures<'a, B, I>(
messages: I,
- round: u64,
- set_id: u64,
+ round: RoundNumber,
+ set_id: SetIdNumber,
mut signatures_checked: usize,
) -> Result<usize, i32> where
B: BlockT,
@@ -919,7 +921,7 @@ impl<Block: BlockT, N: Network<Block>> CommitsOut<Block, N> {
/// Create a new commit output stream.
pub(crate) fn new(
network: N,
- set_id: u64,
+ set_id: SetIdNumber,
is_voter: bool,
gossip_validator: Arc<GossipValidator<Block>>,
) -> Self {
@@ -933,10 +935,10 @@ impl<Block: BlockT, N: Network<Block>> CommitsOut<Block, N> {
}
impl<Block: BlockT, N: Network<Block>> Sink for CommitsOut<Block, N> {
- type SinkItem = (u64, Commit<Block>);
+ type SinkItem = (RoundNumber, Commit<Block>);
type SinkError = Error;
- fn start_send(&mut self, input: (u64, Commit<Block>)) -> StartSend<Self::SinkItem, Error> {
+ fn start_send(&mut self, input: (RoundNumber, Commit<Block>)) -> StartSend<Self::SinkItem, Error> {
if !self.is_voter {
return Ok(AsyncSink::Ready);
}
diff --git a/substrate/core/finality-grandpa/src/environment.rs b/substrate/core/finality-grandpa/src/environment.rs
index b20e8ab5dfc..c0474cb0368 100644
--- a/substrate/core/finality-grandpa/src/environment.rs
+++ b/substrate/core/finality-grandpa/src/environment.rs
@@ -51,7 +51,7 @@ use crate::authorities::{AuthoritySet, SharedAuthoritySet};
use crate::consensus_changes::SharedConsensusChanges;
use crate::justification::GrandpaJustification;
use crate::until_imported::UntilVoteTargetImported;
-use fg_primitives::{AuthorityId, AuthoritySignature};
+use fg_primitives::{AuthorityId, AuthoritySignature, SetId, RoundNumber};
type HistoricalVotes<Block> = grandpa::HistoricalVotes<
<Block as BlockT>::Hash,
@@ -65,7 +65,7 @@ type HistoricalVotes<Block> = grandpa::HistoricalVotes<
#[derive(Debug, Clone, Decode, Encode, PartialEq)]
pub struct CompletedRound<Block: BlockT> {
/// The round number.
- pub number: u64,
+ pub number: RoundNumber,
/// The round state (prevote ghost, estimate, finalized, etc.)
pub state: RoundState<Block::Hash, NumberFor<Block>>,
/// The target block base used for voting in the round.
@@ -80,7 +80,7 @@ pub struct CompletedRound<Block: BlockT> {
#[derive(Debug, Clone, PartialEq)]
pub struct CompletedRounds<Block: BlockT> {
rounds: Vec<CompletedRound<Block>>,
- set_id: u64,
+ set_id: SetId,
voters: Vec<AuthorityId>,
}
@@ -100,7 +100,7 @@ impl<Block: BlockT> codec::EncodeLike for CompletedRounds<Block> {}
impl<Block: BlockT> Decode for CompletedRounds<Block> {
fn decode<I: codec::Input>(value: &mut I) -> Result<Self, codec::Error> {
- <(Vec<CompletedRound<Block>>, u64, Vec<AuthorityId>)>::decode(value)
+ <(Vec<CompletedRound<Block>>, SetId, Vec<AuthorityId>)>::decode(value)
.map(|(rounds, set_id, voters)| CompletedRounds {
rounds: rounds.into(),
set_id,
@@ -113,7 +113,7 @@ impl<Block: BlockT> CompletedRounds<Block> {
/// Create a new completed rounds tracker with NUM_LAST_COMPLETED_ROUNDS capacity.
pub(crate) fn new(
genesis: CompletedRound<Block>,
- set_id: u64,
+ set_id: SetId,
voters: &AuthoritySet<Block::Hash, NumberFor<Block>>,
)
-> CompletedRounds<Block>
@@ -126,7 +126,7 @@ impl<Block: BlockT> CompletedRounds<Block> {
}
/// Get the set-id and voter set of the completed rounds.
- pub fn set_info(&self) -> (u64, &[AuthorityId]) {
+ pub fn set_info(&self) -> (SetId, &[AuthorityId]) {
(self.set_id, &self.voters[..])
}
@@ -162,7 +162,7 @@ impl<Block: BlockT> CompletedRounds<Block> {
/// A map with voter status information for currently live rounds,
/// which votes have we cast and what are they.
-pub type CurrentRounds<Block> = BTreeMap<u64, HasVoted<Block>>;
+pub type CurrentRounds<Block> = BTreeMap<RoundNumber, HasVoted<Block>>;
/// The state of the current voter set, whether it is currently active or not
/// and information related to the previously completed rounds. Current round
@@ -190,7 +190,7 @@ impl<Block: BlockT> VoterSetState<Block> {
/// the given genesis state and the given authorities. Round 1 is added as a
/// current round (with state `HasVoted::No`).
pub(crate) fn live(
- set_id: u64,
+ set_id: SetId,
authority_set: &AuthoritySet<Block::Hash, NumberFor<Block>>,
genesis_state: (Block::Hash, NumberFor<Block>),
) -> VoterSetState<Block> {
@@ -237,7 +237,7 @@ impl<Block: BlockT> VoterSetState<Block> {
/// Returns the voter set state validating that it includes the given round
/// in current rounds and that the voter isn't paused.
- pub fn with_current_round(&self, round: u64)
+ pub fn with_current_round(&self, round: RoundNumber)
-> Result<(&CompletedRounds<Block>, &CurrentRounds<Block>), Error>
{
if let VoterSetState::Live { completed_rounds, current_rounds } = self {
@@ -344,7 +344,7 @@ impl<Block: BlockT> SharedVoterSetState<Block> {
}
/// Return vote status information for the current round.
- pub(crate) fn has_voted(&self, round: u64) -> HasVoted<Block> {
+ pub(crate) fn has_voted(&self, round: RoundNumber) -> HasVoted<Block> {
match &*self.inner.read() {
VoterSetState::Live { current_rounds, .. } => {
current_rounds.get(&round).and_then(|has_voted| match has_voted {
@@ -375,7 +375,7 @@ pub(crate) struct Environment<B, E, Block: BlockT, N: Network<Block>, RA, SC> {
pub(crate) authority_set: SharedAuthoritySet<Block::Hash, NumberFor<Block>>,
pub(crate) consensus_changes: SharedConsensusChanges<Block::Hash, NumberFor<Block>>,
pub(crate) network: crate::communication::NetworkBridge<Block, N>,
- pub(crate) set_id: u64,
+ pub(crate) set_id: SetId,
pub(crate) voter_set_state: SharedVoterSetState<Block>,
}
@@ -554,7 +554,7 @@ where
fn round_data(
&self,
- round: u64
+ round: RoundNumber,
) -> voter::RoundData<Self::Id, Self::Timer, Self::In, Self::Out> {
let now = Instant::now();
let prevote_timer = Delay::new(now + self.config.gossip_duration * 2);
@@ -601,7 +601,7 @@ where
}
}
- fn proposed(&self, round: u64, propose: PrimaryPropose<Block>) -> Result<(), Self::Error> {
+ fn proposed(&self, round: RoundNumber, propose: PrimaryPropose<Block>) -> Result<(), Self::Error> {
let local_id = crate::is_voter(&self.voters, &self.config.keystore);
let local_id = match local_id {
@@ -641,7 +641,7 @@ where
Ok(())
}
- fn prevoted(&self, round: u64, prevote: Prevote<Block>) -> Result<(), Self::Error> {
+ fn prevoted(&self, round: RoundNumber, prevote: Prevote<Block>) -> Result<(), Self::Error> {
let local_id = crate::is_voter(&self.voters, &self.config.keystore);
let local_id = match local_id {
@@ -683,7 +683,7 @@ where
Ok(())
}
- fn precommitted(&self, round: u64, precommit: Precommit<Block>) -> Result<(), Self::Error> {
+ fn precommitted(&self, round: RoundNumber, precommit: Precommit<Block>) -> Result<(), Self::Error> {
let local_id = crate::is_voter(&self.voters, &self.config.keystore);
let local_id = match local_id {
@@ -737,7 +737,7 @@ where
fn completed(
&self,
- round: u64,
+ round: RoundNumber,
state: RoundState<Block::Hash, NumberFor<Block>>,
base: (Block::Hash, NumberFor<Block>),
historical_votes: &HistoricalVotes<Block>,
@@ -794,7 +794,13 @@ where
Ok(())
}
- fn finalize_block(&self, hash: Block::Hash, number: NumberFor<Block>, round: u64, commit: Commit<Block>) -> Result<(), Self::Error> {
+ fn finalize_block(
+ &self,
+ hash: Block::Hash,
+ number: NumberFor<Block>,
+ round: RoundNumber,
+ commit: Commit<Block>,
+ ) -> Result<(), Self::Error> {
finalize_block(
&*self.inner,
&self.authority_set,
@@ -818,7 +824,7 @@ where
fn prevote_equivocation(
&self,
- _round: u64,
+ _round: RoundNumber,
equivocation: ::grandpa::Equivocation<Self::Id, Prevote<Block>, Self::Signature>
) {
warn!(target: "afg", "Detected prevote equivocation in the finality worker: {:?}", equivocation);
@@ -827,7 +833,7 @@ where
fn precommit_equivocation(
&self,
- _round: u64,
+ _round: RoundNumber,
equivocation: Equivocation<Self::Id, Precommit<Block>, Self::Signature>
) {
warn!(target: "afg", "Detected precommit equivocation in the finality worker: {:?}", equivocation);
@@ -837,11 +843,11 @@ where
pub(crate) enum JustificationOrCommit<Block: BlockT> {
Justification(GrandpaJustification<Block>),
- Commit((u64, Commit<Block>)),
+ Commit((RoundNumber, Commit<Block>)),
}
-impl<Block: BlockT> From<(u64, Commit<Block>)> for JustificationOrCommit<Block> {
- fn from(commit: (u64, Commit<Block>)) -> JustificationOrCommit<Block> {
+impl<Block: BlockT> From<(RoundNumber, Commit<Block>)> for JustificationOrCommit<Block> {
+ fn from(commit: (RoundNumber, Commit<Block>)) -> JustificationOrCommit<Block> {
JustificationOrCommit::Commit(commit)
}
}
diff --git a/substrate/core/finality-grandpa/src/lib.rs b/substrate/core/finality-grandpa/src/lib.rs
index f6c11dd6348..b79b120e357 100644
--- a/substrate/core/finality-grandpa/src/lib.rs
+++ b/substrate/core/finality-grandpa/src/lib.rs
@@ -108,7 +108,7 @@ use import::GrandpaBlockImport;
use until_imported::UntilGlobalMessageBlocksImported;
use communication::NetworkBridge;
use service::TelemetryOnConnect;
-use fg_primitives::AuthoritySignature;
+use fg_primitives::{AuthoritySignature, SetId, AuthorityWeight};
// Re-export these two because it's just so damn convenient.
pub use fg_primitives::{AuthorityId, ScheduledChange};
@@ -267,8 +267,8 @@ impl<B, E, Block: BlockT<Hash=H256>, RA> BlockStatus<Block> for Arc<Client<B, E,
pub(crate) struct NewAuthoritySet<H, N> {
pub(crate) canon_number: N,
pub(crate) canon_hash: H,
- pub(crate) set_id: u64,
- pub(crate) authorities: Vec<(AuthorityId, u64)>,
+ pub(crate) set_id: SetId,
+ pub(crate) authorities: Vec<(AuthorityId, AuthorityWeight)>,
}
/// Commands issued to the voter.
@@ -399,7 +399,7 @@ where
}
fn global_communication<Block: BlockT<Hash=H256>, B, E, N, RA>(
- set_id: u64,
+ set_id: SetId,
voters: &Arc<VoterSet<AuthorityId>>,
client: &Arc<Client<B, E, Block, RA>>,
network: &NetworkBridge<Block, N>,
diff --git a/substrate/core/finality-grandpa/src/light_import.rs b/substrate/core/finality-grandpa/src/light_import.rs
index dbdabe96294..4d5381f1cc4 100644
--- a/substrate/core/finality-grandpa/src/light_import.rs
+++ b/substrate/core/finality-grandpa/src/light_import.rs
@@ -36,7 +36,7 @@ use sr_primitives::Justification;
use sr_primitives::traits::{
NumberFor, Block as BlockT, Header as HeaderT, ProvideRuntimeApi, DigestFor,
};
-use fg_primitives::{GrandpaApi, AuthorityId};
+use fg_primitives::{self, GrandpaApi, AuthorityId};
use sr_primitives::generic::BlockId;
use primitives::{H256, Blake2Hasher};
@@ -192,7 +192,7 @@ impl LightAuthoritySet {
/// Get a genesis set with given authorities.
pub fn genesis(initial: Vec<(AuthorityId, u64)>) -> Self {
LightAuthoritySet {
- set_id: 0,
+ set_id: fg_primitives::SetId::default(),
authorities: initial,
}
}
diff --git a/substrate/node/runtime/src/lib.rs b/substrate/node/runtime/src/lib.rs
index 01dff2b7ae8..c8496fa3b15 100644
--- a/substrate/node/runtime/src/lib.rs
+++ b/substrate/node/runtime/src/lib.rs
@@ -80,7 +80,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
// and set impl_version to equal spec_version. If only runtime
// implementation changes and behavior does not, then leave spec_version as
// is and increment impl_version.
- spec_version: 149,
+ spec_version: 150,
impl_version: 150,
apis: RUNTIME_API_VERSIONS,
};
diff --git a/substrate/srml/babe/src/lib.rs b/substrate/srml/babe/src/lib.rs
index b58fb26b5a1..0f439d489e8 100644
--- a/substrate/srml/babe/src/lib.rs
+++ b/substrate/srml/babe/src/lib.rs
@@ -445,6 +445,8 @@ impl<T: Trait> session::OneSessionHandler<T::AccountId> for Module<T> {
fn on_new_session<'a, I: 'a>(_changed: bool, validators: I, queued_validators: I)
where I: Iterator<Item=(&'a T::AccountId, AuthorityId)>
{
+ Self::do_initialize();
+
// Update epoch index
let epoch_index = EpochIndex::get()
.checked_add(1)
diff --git a/substrate/srml/grandpa/src/lib.rs b/substrate/srml/grandpa/src/lib.rs
index d64939ae0a7..5fc354eab96 100644
--- a/substrate/srml/grandpa/src/lib.rs
+++ b/substrate/srml/grandpa/src/lib.rs
@@ -33,7 +33,8 @@ pub use substrate_finality_grandpa_primitives as fg_primitives;
use rstd::prelude::*;
use codec::{self as codec, Encode, Decode, Error};
use srml_support::{
- decl_event, decl_storage, decl_module, dispatch::Result, storage::StorageValue
+ decl_event, decl_storage, decl_module, dispatch::Result,
+ storage::StorageValue, storage::StorageMap,
};
use sr_primitives::{
generic::{DigestItem, OpaqueDigestItemId}, traits::Zero,
@@ -43,7 +44,7 @@ use sr_staking_primitives::{
SessionIndex,
offence::{Offence, Kind},
};
-use fg_primitives::{ScheduledChange, ConsensusLog, GRANDPA_ENGINE_ID};
+use fg_primitives::{GRANDPA_ENGINE_ID, ScheduledChange, ConsensusLog, SetId, RoundNumber};
pub use fg_primitives::{AuthorityId, AuthorityWeight};
use system::{ensure_signed, DigestOf};
@@ -65,7 +66,7 @@ pub struct OldStoredPendingChange<N> {
/// The delay in blocks until it will be applied.
pub delay: N,
/// The next authority set.
- pub next_authorities: Vec<(AuthorityId, u64)>,
+ pub next_authorities: Vec<(AuthorityId, AuthorityWeight)>,
}
/// A stored pending change.
@@ -76,7 +77,7 @@ pub struct StoredPendingChange<N> {
/// The delay in blocks until it will be applied.
pub delay: N,
/// The next authority set.
- pub next_authorities: Vec<(AuthorityId, u64)>,
+ pub next_authorities: Vec<(AuthorityId, AuthorityWeight)>,
/// If defined it means the change was forced and the given block number
/// indicates the median last finalized block when the change was signaled.
pub forced: Option<N>,
@@ -127,7 +128,7 @@ pub enum StoredState<N> {
decl_event!(
pub enum Event {
/// New authority set has been applied.
- NewAuthorities(Vec<(AuthorityId, u64)>),
+ NewAuthorities(Vec<(AuthorityId, AuthorityWeight)>),
/// Current authority set has been paused.
Paused,
/// Current authority set has been resumed.
@@ -151,6 +152,13 @@ decl_storage! {
/// `true` if we are currently stalled.
Stalled get(stalled): Option<(T::BlockNumber, T::BlockNumber)>;
+
+ /// The number of changes (both in terms of keys and underlying economic responsibilities)
+ /// in the "set" of Grandpa validators from genesis.
+ CurrentSetId get(current_set_id) build(|_| fg_primitives::SetId::default()): SetId;
+
+ /// A mapping from grandpa set ID to the index of the *most recent* session for which its members were responsible.
+ SetIdSession get(session_for_set): map SetId => Option<SessionIndex>;
}
add_extra_genesis {
config(authorities): Vec<(AuthorityId, AuthorityWeight)>;
@@ -243,7 +251,7 @@ decl_module! {
impl<T: Trait> Module<T> {
/// Get the current set of authorities, along with their respective weights.
- pub fn grandpa_authorities() -> Vec<(AuthorityId, u64)> {
+ pub fn grandpa_authorities() -> Vec<(AuthorityId, AuthorityWeight)> {
Authorities::get()
}
@@ -292,7 +300,7 @@ impl<T: Trait> Module<T> {
/// No change should be signaled while any change is pending. Returns
/// an error if a change is already pending.
pub fn schedule_change(
- next_authorities: Vec<(AuthorityId, u64)>,
+ next_authorities: Vec<(AuthorityId, AuthorityWeight)>,
in_blocks: T::BlockNumber,
forced: Option<T::BlockNumber>,
) -> Result {
@@ -337,29 +345,34 @@ impl<T: Trait> Module<T> {
}
impl<T: Trait> Module<T> {
+ /// Attempt to extract a GRANDPA log from a generic digest.
pub fn grandpa_log(digest: &DigestOf<T>) -> Option<ConsensusLog<T::BlockNumber>> {
let id = OpaqueDigestItemId::Consensus(&GRANDPA_ENGINE_ID);
digest.convert_first(|l| l.try_to::<ConsensusLog<T::BlockNumber>>(id))
}
+ /// Attempt to extract a pending set-change signal from a digest.
pub fn pending_change(digest: &DigestOf<T>)
-> Option<ScheduledChange<T::BlockNumber>>
{
Self::grandpa_log(digest).and_then(|signal| signal.try_into_change())
}
+ /// Attempt to extract a forced set-change signal from a digest.
pub fn forced_change(digest: &DigestOf<T>)
-> Option<(T::BlockNumber, ScheduledChange<T::BlockNumber>)>
{
Self::grandpa_log(digest).and_then(|signal| signal.try_into_forced_change())
}
+ /// Attempt to extract a pause signal from a digest.
pub fn pending_pause(digest: &DigestOf<T>)
-> Option<T::BlockNumber>
{
Self::grandpa_log(digest).and_then(|signal| signal.try_into_pause())
}
+ /// Attempt to extract a resume signal from a digest.
pub fn pending_resume(digest: &DigestOf<T>)
-> Option<T::BlockNumber>
{
@@ -367,7 +380,9 @@ impl<T: Trait> Module<T> {
}
}
-impl<T: Trait> session::OneSessionHandler<T::AccountId> for Module<T> {
+impl<T: Trait> session::OneSessionHandler<T::AccountId> for Module<T>
+ where T: session::Trait
+{
type Key = AuthorityId;
fn on_genesis_session<'a, I: 'a>(validators: I)
@@ -380,18 +395,27 @@ impl<T: Trait> session::OneSessionHandler<T::AccountId> for Module<T> {
fn on_new_session<'a, I: 'a>(changed: bool, validators: I, _queued_validators: I)
where I: Iterator<Item=(&'a T::AccountId, AuthorityId)>
{
- // instant changes
- if changed {
+ // Always issue a change if `session` says that the validators have changed.
+ // Even if their session keys are the same as before, the underyling economic
+ // identities have changed.
+ let current_set_id = if changed {
let next_authorities = validators.map(|(_, k)| (k, 1)).collect::<Vec<_>>();
- let last_authorities = <Module<T>>::grandpa_authorities();
- if next_authorities != last_authorities {
- if let Some((further_wait, median)) = <Stalled<T>>::take() {
- let _ = Self::schedule_change(next_authorities, further_wait, Some(median));
- } else {
- let _ = Self::schedule_change(next_authorities, Zero::zero(), None);
- }
+ if let Some((further_wait, median)) = <Stalled<T>>::take() {
+ let _ = Self::schedule_change(next_authorities, further_wait, Some(median));
+ } else {
+ let _ = Self::schedule_change(next_authorities, Zero::zero(), None);
}
- }
+ CurrentSetId::mutate(|s| { *s += 1; *s })
+ } else {
+ // nothing's changed, neither economic conditions nor session keys. update the pointer
+ // of the current set.
+ Self::current_set_id()
+ };
+
+ // if we didn't issue a change, we update the mapping to note that the current
+ // set corresponds to the latest equivalent session (i.e. now).
+ let session_index = <session::Module<T>>::current_index();
+ SetIdSession::insert(current_set_id, &session_index);
}
fn on_disabled(i: usize) {
@@ -412,8 +436,8 @@ impl<T: Trait> finality_tracker::OnFinalizationStalled<T::BlockNumber> for Modul
#[derive(Copy, Clone, PartialOrd, Ord, Eq, PartialEq, Encode, Decode)]
struct GrandpaTimeSlot {
// The order of these matters for `derive(Ord)`.
- set_id: u64,
- round: u64,
+ set_id: SetId,
+ round: RoundNumber,
}
// TODO [slashing]: Integrate this.
diff --git a/substrate/srml/session/src/lib.rs b/substrate/srml/session/src/lib.rs
index f8e13d529b8..6a09591e014 100644
--- a/substrate/srml/session/src/lib.rs
+++ b/substrate/srml/session/src/lib.rs
@@ -170,6 +170,12 @@ impl<
pub trait OnSessionEnding<ValidatorId> {
/// Handle the fact that the session is ending, and optionally provide the new validator set.
///
+ /// Even if the validator-set is the same as before, if any underlying economic
+ /// conditions have changed (i.e. stake-weights), the new validator set must be returned.
+ /// This is necessary for consensus engines making use of the session module to
+ /// issue a validator-set change so misbehavior can be provably associated with the new
+ /// economic conditions as opposed to the old.
+ ///
/// `ending_index` is the index of the currently ending session.
/// The returned validator set, if any, will not be applied until `will_apply_at`.
/// `will_apply_at` is guaranteed to be at least `ending_index + 1`, since session indices don't
@@ -192,7 +198,11 @@ pub trait SessionHandler<ValidatorId> {
/// should provide the same validator set.
fn on_genesis_session<Ks: OpaqueKeys>(validators: &[(ValidatorId, Ks)]);
- /// Session set has changed; act appropriately.
+ /// Session set has changed; act appropriately. Note that this can be called
+ /// before initialization of your module.
+ ///
+ /// `changed` is true whenever any of the session keys or underlying economic
+ /// identities or weightings behind those keys has changed.
fn on_new_session<Ks: OpaqueKeys>(
changed: bool,
validators: &[(ValidatorId, Ks)],
@@ -217,11 +227,19 @@ pub trait OneSessionHandler<ValidatorId> {
fn on_genesis_session<'a, I: 'a>(validators: I)
where I: Iterator<Item=(&'a ValidatorId, Self::Key)>, ValidatorId: 'a;
- /// Session set has changed; act appropriately.
+ /// Session set has changed; act appropriately. Note that this can be called
+ /// before initialization of your module.
+ ///
+ /// `changed` is true when at least one of the session keys
+ /// or the underlying economic identities/distribution behind one the
+ /// session keys has changed, false otherwise.
+ ///
+ /// The `validators` are the validators of the incoming session, and `queued_validators`
+ /// will follow.
fn on_new_session<'a, I: 'a>(
- _changed: bool,
- _validators: I,
- _queued_validators: I
+ changed: bool,
+ validators: I,
+ queued_validators: I,
) where I: Iterator<Item=(&'a ValidatorId, Self::Key)>, ValidatorId: 'a;
@@ -341,10 +359,8 @@ decl_storage! {
/// Current index of the session.
CurrentIndex get(current_index): SessionIndex;
- /// True if anything has changed in this session.
- Changed: bool;
-
- /// Queued keys changed.
+ /// True if the underlying economic identities or weighting behind the validators
+ /// has changed in the queued validator set.
QueuedChanged: bool;
/// The queued keys for the next session. When the next session begins, these keys
@@ -443,13 +459,10 @@ decl_module! {
Self::do_set_keys(&who, keys)?;
- // Something changed.
- Changed::put(true);
-
Ok(())
}
- /// Called when a block is finalized. Will rotate session if it is the last
+ /// Called when a block is initialized. Will rotate session if it is the last
/// block of the current session.
fn on_initialize(n: T::BlockNumber) {
if T::ShouldEndSession::should_end_session(n) {
@@ -467,7 +480,6 @@ impl<T: Trait> Module<T> {
let session_index = CurrentIndex::get();
let changed = QueuedChanged::get();
- let mut next_changed = Changed::take();
// Get queued session keys and validators.
let session_keys = <QueuedKeys<T>>::get();
@@ -479,12 +491,16 @@ impl<T: Trait> Module<T> {
let applied_at = session_index + 2;
// Get next validator set.
- let maybe_validators = T::OnSessionEnding::on_session_ending(session_index, applied_at);
- let next_validators = if let Some(validators) = maybe_validators {
- next_changed = true;
- validators
+ let maybe_next_validators = T::OnSessionEnding::on_session_ending(session_index, applied_at);
+ let (next_validators, next_identities_changed)
+ = if let Some(validators) = maybe_next_validators
+ {
+ // NOTE: as per the documentation on `OnSessionEnding`, we consider
+ // the validator set as having changed even if the validators are the
+ // same as before, as underlying economic conditions may have changed.
+ (validators, true)
} else {
- <Validators<T>>::get()
+ (<Validators<T>>::get(), false)
};
// Increment session index.
@@ -492,9 +508,34 @@ impl<T: Trait> Module<T> {
CurrentIndex::put(session_index);
// Queue next session keys.
- let queued_amalgamated = next_validators.into_iter()
- .map(|a| { let k = Self::load_keys(&a).unwrap_or_default(); (a, k) })
- .collect::<Vec<_>>();
+ let (queued_amalgamated, next_changed) = {
+ // until we are certain there has been a change, iterate the prior
+ // validators along with the current and check for changes
+ let mut changed = next_identities_changed;
+
+ let mut now_session_keys = session_keys.iter();
+ let mut check_next_changed = |keys: &T::Keys| {
+ if changed { return }
+ // since a new validator set always leads to `changed` starting
+ // as true, we can ensure that `now_session_keys` and `next_validators`
+ // have the same length. this function is called once per iteration.
+ if let Some(&(_, ref old_keys)) = now_session_keys.next() {
+ if old_keys != keys {
+ changed = true;
+ return
+ }
+ }
+ };
+ let queued_amalgamated = next_validators.into_iter()
+ .map(|a| {
+ let k = Self::load_keys(&a).unwrap_or_default();
+ check_next_changed(&k);
+ (a, k)
+ })
+ .collect::<Vec<_>>();
+
+ (queued_amalgamated, changed)
+ };
<QueuedKeys<T>>::put(queued_amalgamated.clone());
QueuedChanged::put(next_changed);
@@ -503,13 +544,16 @@ impl<T: Trait> Module<T> {
Self::deposit_event(Event::NewSession(session_index));
// Tell everyone about the new session keys.
- T::SessionHandler::on_new_session::<T::Keys>(changed, &session_keys, &queued_amalgamated);
+ T::SessionHandler::on_new_session::<T::Keys>(
+ changed,
+ &session_keys,
+ &queued_amalgamated,
+ );
}
/// Disable the validator of index `i`.
pub fn disable_index(i: usize) {
T::SessionHandler::on_disabled(i);
- Changed::put(true);
}
/// Disable the validator identified by `c`. (If using with the staking module, this would be
@@ -554,8 +598,6 @@ impl<T: Trait> Module<T> {
let key_data = old_keys.get_raw(id);
Self::clear_key_owner(id, key_data);
}
-
- Changed::put(true);
}
}
@@ -668,8 +710,6 @@ mod tests {
Session::on_free_balance_zero(&1);
assert_eq!(Session::load_keys(&1), None);
assert_eq!(Session::key_owner(id, UintAuthorityId(1).get_raw(id)), None);
-
- assert!(Changed::get());
})
}
@@ -816,9 +856,16 @@ mod tests {
initialize_block(6);
assert!(!session_changed());
+ // changing the keys of a validator leads to change.
+ assert_ok!(Session::set_keys(Origin::signed(69), UintAuthorityId(69).into(), vec![]));
force_new_session();
initialize_block(7);
assert!(session_changed());
+
+ // while changing the keys of a non-validator does not.
+ force_new_session();
+ initialize_block(7);
+ assert!(!session_changed());
});
}
diff --git a/substrate/srml/session/src/mock.rs b/substrate/srml/session/src/mock.rs
index c5608e1a545..13fcb54754c 100644
--- a/substrate/srml/session/src/mock.rs
+++ b/substrate/srml/session/src/mock.rs
@@ -44,6 +44,7 @@ impl_outer_origin! {
}
thread_local! {
+ pub static VALIDATORS: RefCell<Vec<u64>> = RefCell::new(vec![1, 2, 3]);
pub static NEXT_VALIDATORS: RefCell<Vec<u64>> = RefCell::new(vec![1, 2, 3]);
pub static AUTHORITIES: RefCell<Vec<UintAuthorityId>> =
RefCell::new(vec![UintAuthorityId(1), UintAuthorityId(2), UintAuthorityId(3)]);
@@ -51,6 +52,7 @@ thread_local! {
pub static SESSION_LENGTH: RefCell<u64> = RefCell::new(2);
pub static SESSION_CHANGED: RefCell<bool> = RefCell::new(false);
pub static TEST_SESSION_CHANGED: RefCell<bool> = RefCell::new(false);
+ pub static DISABLED: RefCell<bool> = RefCell::new(false);
}
pub struct TestShouldEndSession;
@@ -76,14 +78,24 @@ impl SessionHandler<u64> for TestSessionHandler {
.collect()
);
}
- fn on_disabled(_validator_index: usize) {}
+ fn on_disabled(_validator_index: usize) {
+ DISABLED.with(|l| *l.borrow_mut() = true)
+ }
}
pub struct TestOnSessionEnding;
impl OnSessionEnding<u64> for TestOnSessionEnding {
fn on_session_ending(_: SessionIndex, _: SessionIndex) -> Option<Vec<u64>> {
if !TEST_SESSION_CHANGED.with(|l| *l.borrow()) {
- Some(NEXT_VALIDATORS.with(|l| l.borrow().clone()))
+ VALIDATORS.with(|v| {
+ let mut v = v.borrow_mut();
+ *v = NEXT_VALIDATORS.with(|l| l.borrow().clone());
+ Some(v.clone())
+ })
+ } else if DISABLED.with(|l| std::mem::replace(&mut *l.borrow_mut(), false)) {
+ // If there was a disabled validator, underlying conditions have changed
+ // so we return `Some`.
+ Some(VALIDATORS.with(|v| v.borrow().clone()))
} else {
None
}
@@ -92,16 +104,13 @@ impl OnSessionEnding<u64> for TestOnSessionEnding {
#[cfg(feature = "historical")]
impl crate::historical::OnSessionEnding<u64, u64> for TestOnSessionEnding {
- fn on_session_ending(_: SessionIndex, _: SessionIndex)
+ fn on_session_ending(ending_index: SessionIndex, will_apply_at: SessionIndex)
-> Option<(Vec<u64>, Vec<(u64, u64)>)>
{
- if !TEST_SESSION_CHANGED.with(|l| *l.borrow()) {
- let last_validators = Session::validators();
- let last_identifications = last_validators.into_iter().map(|v| (v, v)).collect();
- Some((NEXT_VALIDATORS.with(|l| l.borrow().clone()), last_identifications))
- } else {
- None
- }
+ let pair_with_ids = |vals: &[u64]| vals.iter().map(|&v| (v, v)).collect::<Vec<_>>();
+ <Self as OnSessionEnding<_>>::on_session_ending(ending_index, will_apply_at)
+ .map(|vals| (pair_with_ids(&vals), vals))
+ .map(|(ids, vals)| (vals, ids))
}
}
diff --git a/substrate/srml/staking/src/lib.rs b/substrate/srml/staking/src/lib.rs
index e7bb42c64df..9386035e44e 100644
--- a/substrate/srml/staking/src/lib.rs
+++ b/substrate/srml/staking/src/lib.rs
@@ -1355,6 +1355,10 @@ impl<T: Trait> Module<T> {
// Set the new validator set in sessions.
<CurrentElected<T>>::put(&elected_stashes);
+ // In order to keep the property required by `n_session_ending`
+ // that we must return the new validator set even if it's the same as the old,
+ // as long as any underlying economic conditions have changed, we don't attempt
+ // to do any optimization where we compare against the prior set.
(slot_stake, Some(elected_stashes))
} else {
// There were not enough candidates for even our minimal level of functionality.
--
GitLab