grandpa: track multiple live rounds in voter set state (#3298)

* grandpa: track multiple live rounds in voter set state * grandpa: don't assume rounds are completed in-order * grandpa: fix tests * grandpa: don't assume round is being tracked on Environment::completed * grandpa: fix missing import in test
2026-07-20 08:05:41 +00:00 · 2019-08-08 09:56:49 +02:00
parent 0f0df9850a
commit 6565e1f8aa
7 changed files with 247 additions and 205 deletions
@@ -14,7 +14,7 @@
 // You should have received a copy of the GNU General Public License
 // along with Substrate.  If not, see <http://www.gnu.org/licenses/>.

-use std::collections::VecDeque;
+use std::collections::BTreeMap;
 use std::iter::FromIterator;
 use std::sync::Arc;
 use std::time::{Duration, Instant};
@@ -74,11 +74,12 @@ pub struct CompletedRound<Block: BlockT> {
 	pub votes: Vec<SignedMessage<Block>>,
 }

-// Data about last completed rounds within a single voter set. Stores NUM_LAST_COMPLETED_ROUNDS and always
-// contains data about at least one round (genesis).
+// Data about last completed rounds within a single voter set. Stores
+// NUM_LAST_COMPLETED_ROUNDS and always contains data about at least one round
+// (genesis).
 #[derive(Debug, Clone, PartialEq)]
 pub struct CompletedRounds<Block: BlockT> {
-	rounds: VecDeque<CompletedRound<Block>>,
+	rounds: Vec<CompletedRound<Block>>,
 	set_id: u64,
 	voters: Vec<AuthorityId>,
 }
@@ -117,8 +118,8 @@ impl<Block: BlockT> CompletedRounds<Block> {
 	)
 		-> CompletedRounds<Block>
 	{
-		let mut rounds = VecDeque::with_capacity(NUM_LAST_COMPLETED_ROUNDS);
-		rounds.push_back(genesis);
+		let mut rounds = Vec::with_capacity(NUM_LAST_COMPLETED_ROUNDS);
+		rounds.push(genesis);

 		let voters = voters.current().1.iter().map(|(a, _)| a.clone()).collect();
 		CompletedRounds { rounds, set_id, voters }
@@ -131,32 +132,38 @@ impl<Block: BlockT> CompletedRounds<Block> {

 	/// Iterate over all completed rounds.
 	pub fn iter(&self) -> impl Iterator<Item=&CompletedRound<Block>> {
-		self.rounds.iter()
+		self.rounds.iter().rev()
 	}

 	/// Returns the last (latest) completed round.
 	pub fn last(&self) -> &CompletedRound<Block> {
-		self.rounds.back()
+		self.rounds.first()
 			.expect("inner is never empty; always contains at least genesis; qed")
 	}

-	/// Push a new completed round, returns false if the given round is older
-	/// than the last completed round.
-	pub fn push(&mut self, completed_round: CompletedRound<Block>) -> bool {
-		if self.last().number >= completed_round.number {
-			return false;
+	/// Push a new completed round, oldest round is evicted if number of rounds
+	/// is higher than `NUM_LAST_COMPLETED_ROUNDS`.
+	pub fn push(&mut self, completed_round: CompletedRound<Block>) {
+		use std::cmp::Reverse;
+
+		match self.rounds.binary_search_by_key(
+			&Reverse(completed_round.number),
+			|completed_round| Reverse(completed_round.number),
+		) {
+			Ok(idx) => self.rounds[idx] = completed_round,
+			Err(idx) => self.rounds.insert(idx, completed_round),
+		};
+
+		if self.rounds.len() > NUM_LAST_COMPLETED_ROUNDS {
+			self.rounds.pop();
 		}
-
-		if self.rounds.len() == NUM_LAST_COMPLETED_ROUNDS {
-			self.rounds.pop_front();
-		}
-
-		self.rounds.push_back(completed_round);
-
-		true
 	}
 }

+/// 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>>;
+
 /// The state of the current voter set, whether it is currently active or not
 /// and information related to the previously completed rounds. Current round
 /// voting status is used when restarting the voter, i.e. it will re-use the
@@ -168,8 +175,8 @@ pub enum VoterSetState<Block: BlockT> {
 	Live {
 		/// The previously completed rounds.
 		completed_rounds: CompletedRounds<Block>,
-		/// Vote status for the current round.
-		current_round: HasVoted<Block>,
+		/// Voter status for the currently live rounds.
+		current_rounds: CurrentRounds<Block>,
 	},
 	/// The voter is paused, i.e. not casting or importing any votes.
 	Paused {
@@ -179,6 +186,35 @@ pub enum VoterSetState<Block: BlockT> {
 }

 impl<Block: BlockT> VoterSetState<Block> {
+	/// Create a new live VoterSetState with round 0 as a completed round using
+	/// 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,
+		authority_set: &AuthoritySet<Block::Hash, NumberFor<Block>>,
+		genesis_state: (Block::Hash, NumberFor<Block>),
+	) -> VoterSetState<Block> {
+		let state = RoundState::genesis((genesis_state.0, genesis_state.1));
+		let completed_rounds = CompletedRounds::new(
+			CompletedRound {
+				number: 0,
+				state,
+				base: (genesis_state.0, genesis_state.1),
+				votes: Vec::new(),
+			},
+			set_id,
+			authority_set,
+		);
+
+		let mut current_rounds = CurrentRounds::new();
+		current_rounds.insert(1, HasVoted::No);
+
+		VoterSetState::Live {
+			completed_rounds,
+			current_rounds,
+		}
+	}
+
 	/// Returns the last completed rounds.
 	pub(crate) fn completed_rounds(&self) -> CompletedRounds<Block> {
 		match self {
@@ -198,10 +234,28 @@ impl<Block: BlockT> VoterSetState<Block> {
 				completed_rounds.last().clone(),
 		}
 	}
+
+	/// 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)
+		-> Result<(&CompletedRounds<Block>, &CurrentRounds<Block>), Error>
+	{
+		if let VoterSetState::Live { completed_rounds, current_rounds } = self {
+			if current_rounds.contains_key(&round) {
+				return Ok((completed_rounds, current_rounds));
+			} else {
+				let msg = "Voter acting on a live round we are not tracking.";
+				return Err(Error::Safety(msg.to_string()));
+			}
+		} else {
+			let msg = "Voter acting while in paused state.";
+			return Err(Error::Safety(msg.to_string()));
+		}
+	}
 }

 /// Whether we've voted already during a prior run of the program.
-#[derive(Debug, Decode, Encode, PartialEq)]
+#[derive(Clone, Debug, Decode, Encode, PartialEq)]
 pub enum HasVoted<Block: BlockT> {
 	/// Has not voted already in this round.
 	No,
@@ -290,10 +344,16 @@ impl<Block: BlockT> SharedVoterSetState<Block> {
 	}

 	/// Return vote status information for the current round.
-	pub(crate) fn has_voted(&self) -> HasVoted<Block> {
+	pub(crate) fn has_voted(&self, round: u64) -> HasVoted<Block> {
 		match &*self.inner.read() {
-			VoterSetState::Live { current_round: HasVoted::Yes(id, vote), .. } =>
-				HasVoted::Yes(id.clone(), vote.clone()),
+			VoterSetState::Live { current_rounds, .. } => {
+				current_rounds.get(&round).and_then(|has_voted| match has_voted {
+					HasVoted::Yes(id, vote) =>
+						Some(HasVoted::Yes(id.clone(), vote.clone())),
+					_ => None,
+				})
+				.unwrap_or(HasVoted::No)
+			},
 			_ => HasVoted::No,
 		}
 	}
@@ -502,7 +562,7 @@ where

 		let local_key = crate::is_voter(&self.voters, &self.config.keystore);

-		let has_voted = match self.voter_set_state.has_voted() {
+		let has_voted = match self.voter_set_state.has_voted(round) {
 			HasVoted::Yes(id, vote) => {
 				if local_key.as_ref().map(|k| k.public() == id).unwrap_or(false) {
 					HasVoted::Yes(id, vote)
@@ -541,7 +601,7 @@ where
 		}
 	}

-	fn proposed(&self, _round: u64, propose: PrimaryPropose<Block>) -> Result<(), Self::Error> {
+	fn proposed(&self, round: u64, propose: PrimaryPropose<Block>) -> Result<(), Self::Error> {
 		let local_id = crate::is_voter(&self.voters, &self.config.keystore);

 		let local_id = match local_id {
@@ -550,23 +610,26 @@ where
 		};

 		self.update_voter_set_state(|voter_set_state| {
-			let completed_rounds = match voter_set_state {
-				VoterSetState::Live { completed_rounds, current_round: HasVoted::No } => completed_rounds,
-				VoterSetState::Live { current_round, .. } if !current_round.can_propose() => {
-					// we've already proposed in this round (in a previous run),
-					// ignore the given vote and don't update the voter set
-					// state
-					return Ok(None);
-				},
-				_ => {
-					let msg = "Voter proposing after prevote/precommit or while paused.";
-					return Err(Error::Safety(msg.to_string()));
-				},
-			};
+			let (completed_rounds, current_rounds) = voter_set_state.with_current_round(round)?;
+			let current_round = current_rounds.get(&round)
+				.expect("checked in with_current_round that key exists; qed.");
+
+			if !current_round.can_propose() {
+				// we've already proposed in this round (in a previous run),
+				// ignore the given vote and don't update the voter set
+				// state
+				return Ok(None);
+			}
+
+			let mut current_rounds = current_rounds.clone();
+			let current_round = current_rounds.get_mut(&round)
+				.expect("checked previously that key exists; qed.");
+
+			*current_round = HasVoted::Yes(local_id, Vote::Propose(propose));

 			let set_state = VoterSetState::<Block>::Live {
 				completed_rounds: completed_rounds.clone(),
-				current_round: HasVoted::Yes(local_id, Vote::Propose(propose)),
+				current_rounds,
 			};

 			#[allow(deprecated)]
@@ -578,7 +641,7 @@ where
 		Ok(())
 	}

-	fn prevoted(&self, _round: u64, prevote: Prevote<Block>) -> Result<(), Self::Error> {
+	fn prevoted(&self, round: u64, prevote: Prevote<Block>) -> Result<(), Self::Error> {
 		let local_id = crate::is_voter(&self.voters, &self.config.keystore);

 		let local_id = match local_id {
@@ -587,26 +650,28 @@ where
 		};

 		self.update_voter_set_state(|voter_set_state| {
-			let (completed_rounds, propose) = match voter_set_state {
-				VoterSetState::Live { completed_rounds, current_round: HasVoted::No } =>
-					(completed_rounds, None),
-				VoterSetState::Live { completed_rounds, current_round: HasVoted::Yes(_, Vote::Propose(propose)) } =>
-					(completed_rounds, Some(propose)),
-				VoterSetState::Live { current_round, .. } if !current_round.can_prevote() => {
-					// we've already prevoted in this round (in a previous run),
-					// ignore the given vote and don't update the voter set
-					// state
-					return Ok(None);
-				},
-				_ => {
-					let msg = "Voter prevoting after precommit or while paused.";
-					return Err(Error::Safety(msg.to_string()));
-				},
-			};
+			let (completed_rounds, current_rounds) = voter_set_state.with_current_round(round)?;
+			let current_round = current_rounds.get(&round)
+				.expect("checked in with_current_round that key exists; qed.");
+
+			if !current_round.can_prevote() {
+				// we've already prevoted in this round (in a previous run),
+				// ignore the given vote and don't update the voter set
+				// state
+				return Ok(None);
+			}
+
+			let propose = current_round.propose();
+
+			let mut current_rounds = current_rounds.clone();
+			let current_round = current_rounds.get_mut(&round)
+				.expect("checked previously that key exists; qed.");
+
+			*current_round = HasVoted::Yes(local_id, Vote::Prevote(propose.cloned(), prevote));

 			let set_state = VoterSetState::<Block>::Live {
 				completed_rounds: completed_rounds.clone(),
-				current_round: HasVoted::Yes(local_id, Vote::Prevote(propose.cloned(), prevote)),
+				current_rounds,
 			};

 			#[allow(deprecated)]
@@ -618,7 +683,7 @@ where
 		Ok(())
 	}

-	fn precommitted(&self, _round: u64, precommit: Precommit<Block>) -> Result<(), Self::Error> {
+	fn precommitted(&self, round: u64, precommit: Precommit<Block>) -> Result<(), Self::Error> {
 		let local_id = crate::is_voter(&self.voters, &self.config.keystore);

 		let local_id = match local_id {
@@ -627,24 +692,38 @@ where
 		};

 		self.update_voter_set_state(|voter_set_state| {
-			let (completed_rounds, propose, prevote) = match voter_set_state {
-				VoterSetState::Live { completed_rounds, current_round: HasVoted::Yes(_, Vote::Prevote(propose, prevote)) } =>
-					(completed_rounds, propose, prevote),
-				VoterSetState::Live { current_round, .. } if !current_round.can_precommit() => {
-					// we've already precommitted in this round (in a previous run),
-					// ignore the given vote and don't update the voter set
-					// state
-					return Ok(None);
-				},
+			let (completed_rounds, current_rounds) = voter_set_state.with_current_round(round)?;
+			let current_round = current_rounds.get(&round)
+				.expect("checked in with_current_round that key exists; qed.");
+
+			if !current_round.can_precommit() {
+				// we've already precommitted in this round (in a previous run),
+				// ignore the given vote and don't update the voter set
+				// state
+				return Ok(None);
+			}
+
+			let propose = current_round.propose();
+			let prevote = match current_round {
+				HasVoted::Yes(_, Vote::Prevote(_, prevote)) => prevote,
 				_ => {
-					let msg = "Voter precommitting while paused.";
+					let msg = "Voter precommitting before prevoting.";
 					return Err(Error::Safety(msg.to_string()));
-				}
+				},
 			};

+			let mut current_rounds = current_rounds.clone();
+			let current_round = current_rounds.get_mut(&round)
+				.expect("checked previously that key exists; qed.");
+
+			*current_round = HasVoted::Yes(
+				local_id,
+				Vote::Precommit(propose.cloned(), prevote.clone(), precommit),
+			);
+
 			let set_state = VoterSetState::<Block>::Live {
 				completed_rounds: completed_rounds.clone(),
-				current_round: HasVoted::Yes(local_id, Vote::Precommit(propose.clone(), prevote.clone(), precommit)),
+				current_rounds,
 			};

 			#[allow(deprecated)]
@@ -673,25 +752,37 @@ where
 		);

 		self.update_voter_set_state(|voter_set_state| {
-			let mut completed_rounds = voter_set_state.completed_rounds();
+			// NOTE: we don't use `with_current_round` here, it is possible that
+			// we are not currently tracking this round if it is a round we
+			// caught up to.
+			let (completed_rounds, current_rounds) =
+				if let VoterSetState::Live { completed_rounds, current_rounds } = voter_set_state {
+					(completed_rounds, current_rounds)
+				} else {
+					let msg = "Voter acting while in paused state.";
+					return Err(Error::Safety(msg.to_string()));
+				};
+
+			let mut completed_rounds = completed_rounds.clone();

 			// TODO: Future integration will store the prevote and precommit index. See #2611.
 			let votes = historical_votes.seen().clone();

-			// NOTE: the Environment assumes that rounds are *always* completed in-order.
-			if !completed_rounds.push(CompletedRound {
+			completed_rounds.push(CompletedRound {
 				number: round,
 				state: state.clone(),
 				base,
 				votes,
-			}) {
-				let msg = "Voter completed round that is older than the last completed round.";
-				return Err(Error::Safety(msg.to_string()));
-			};
+			});
+
+			// remove the round from live rounds and start tracking the next round
+			let mut current_rounds = current_rounds.clone();
+			current_rounds.remove(&round);
+			current_rounds.insert(round + 1, HasVoted::No);

 			let set_state = VoterSetState::<Block>::Live {
 				completed_rounds,
-				current_round: HasVoted::No,
+				current_rounds,
 			};

 			#[allow(deprecated)]