apollo_consensus: SM tracks last self votes not the SHC

Author: asmaastarkware

crates/apollo_consensus/src/single_height_consensus.rs

@@ -139,8 +139,6 @@ pub(crate) struct SingleHeightConsensus {
     state_machine: StateMachine,
     // Tracks rounds for which we started validating a proposal to avoid duplicate validations.
     pending_validation_rounds: HashSet<Round>,
-    last_prevote: Option<Vote>,
-    last_precommit: Option<Vote>,
     height_voted_storage: Arc<Mutex<dyn HeightVotedStorageTrait>>,
 }
 
@@ -163,8 +161,6 @@ impl SingleHeightConsensus {
             timeouts,
             state_machine,
             pending_validation_rounds: HashSet::new(),
-            last_prevote: None,
-            last_precommit: None,
             height_voted_storage,
         }
     }
@@ -247,7 +243,7 @@ impl SingleHeightConsensus {
                 self.handle_timeout(context, event).await
             }
             StateMachineEvent::Prevote(vote) => {
-                let Some(last_vote) = &self.last_prevote else {
+                let Some(last_vote) = self.state_machine.last_self_prevote() else {
                     return Err(ConsensusError::InternalInconsistency(
                         "No prevote to send".to_string(),
                     ));
@@ -264,7 +260,7 @@ impl SingleHeightConsensus {
                 )]))
             }
             StateMachineEvent::Precommit(vote) => {
-                let Some(last_vote) = &self.last_precommit else {
+                let Some(last_vote) = self.state_machine.last_self_precommit() else {
                     return Err(ConsensusError::InternalInconsistency(
                         "No precommit to send".to_string(),
                     ));
@@ -503,37 +499,18 @@ impl SingleHeightConsensus {
         context: &mut ContextT,
         vote: Vote,
     ) -> Result<Vec<ShcTask>, ConsensusError> {
-        let (last_vote, task) = match vote.vote_type {
-            VoteType::Prevote => (
-                &mut self.last_prevote,
-                ShcTask::Prevote(
-                    self.timeouts.get_prevote_timeout(0),
-                    StateMachineEvent::Prevote(vote.clone()),
-                ),
+        let task = match vote.vote_type {
+            VoteType::Prevote => ShcTask::Prevote(
+                self.timeouts.get_prevote_timeout(0),
+                StateMachineEvent::Prevote(vote.clone()),
             ),
-            VoteType::Precommit => (
-                &mut self.last_precommit,
-                ShcTask::Precommit(
-                    self.timeouts.get_precommit_timeout(0),
-                    StateMachineEvent::Precommit(vote.clone()),
-                ),
+            VoteType::Precommit => ShcTask::Precommit(
+                self.timeouts.get_precommit_timeout(0),
+                StateMachineEvent::Precommit(vote.clone()),
             ),
         };
         // Ensure the voter matches this node.
         assert_eq!(vote.voter, self.state_machine.validator_id());
-        *last_vote = match last_vote {
-            None => Some(vote.clone()),
-            Some(last_vote) if vote.round > last_vote.round => Some(vote.clone()),
-            Some(_) => {
-                // According to the Tendermint paper, the state machine should only vote for its
-                // current round. It should monotonically increase its round. It should only vote
-                // once per step.
-                return Err(ConsensusError::InternalInconsistency(format!(
-                    "State machine must progress in time: last_vote: {last_vote:?} new_vote: \
-                     {vote:?}",
-                )));
-            }
-        };
 
         trace!("Writing voted height {} to storage", self.state_machine.height());
         self.height_voted_storage

crates/apollo_consensus/src/state_machine.rs

@@ -99,6 +99,9 @@ pub(crate) struct StateMachine {
     prevote_quorum: HashSet<Round>,
     mixed_prevote_quorum: HashSet<Round>,
     mixed_precommit_quorum: HashSet<Round>,
+    // Tracks the latest self votes for efficient rebroadcasts.
+    last_self_prevote: Option<Vote>,
+    last_self_precommit: Option<Vote>,
 }
 
 impl StateMachine {
@@ -131,6 +134,8 @@ impl StateMachine {
             prevote_quorum: HashSet::new(),
             mixed_prevote_quorum: HashSet::new(),
             mixed_precommit_quorum: HashSet::new(),
+            last_self_prevote: None,
+            last_self_precommit: None,
         }
     }
 
@@ -170,13 +175,37 @@ impl StateMachine {
         self.proposals.get(&round).and_then(|(id, _)| *id)
     }
 
+    pub(crate) fn last_self_prevote(&self) -> Option<Vote> {
+        self.last_self_prevote.clone()
+    }
+
+    pub(crate) fn last_self_precommit(&self) -> Option<Vote> {
+        self.last_self_precommit.clone()
+    }
+
     fn make_self_vote(
-        &self,
+        &mut self,
         vote_type: VoteType,
-        round: Round,
         proposal_commitment: Option<ProposalCommitment>,
     ) -> Vote {
-        Vote { vote_type, height: self.height.0, round, proposal_commitment, voter: self.id }
+        let vote = Vote {
+            vote_type,
+            height: self.height.0,
+            round: self.round,
+            proposal_commitment,
+            voter: self.id,
+        };
+        // update the latest self vote.
+        let last_self_vote = match vote_type {
+            VoteType::Prevote => &mut self.last_self_prevote,
+            VoteType::Precommit => &mut self.last_self_precommit,
+        };
+        assert!(
+            last_self_vote.as_ref().is_none_or(|last| self.round > last.round),
+            "State machine must progress in time: last_vote: {last_self_vote:?} new_vote: {vote:?}"
+        );
+        *last_self_vote = Some(vote.clone());
+        vote
     }
 
     /// Starts the state machine, effectively calling `StartRound(0)` from the paper. This is
@@ -338,7 +367,7 @@ impl StateMachine {
              round={round}."
         );
         CONSENSUS_TIMEOUTS.increment(1, &[(LABEL_NAME_TIMEOUT_TYPE, TimeoutType::Propose.into())]);
-        let vote = self.make_self_vote(VoteType::Prevote, round, None);
+        let vote = self.make_self_vote(VoteType::Prevote, None);
         let mut output = VecDeque::from([StateMachineEvent::Prevote(vote)]);
         output.append(&mut self.advance_to_step(Step::Prevote));
         output
@@ -370,7 +399,7 @@ impl StateMachine {
         };
         debug!("Applying TimeoutPrevote for round={round}.");
         CONSENSUS_TIMEOUTS.increment(1, &[(LABEL_NAME_TIMEOUT_TYPE, TimeoutType::Prevote.into())]);
-        let vote = self.make_self_vote(VoteType::Precommit, round, None);
+        let vote = self.make_self_vote(VoteType::Precommit, None);
         let mut output = VecDeque::from([StateMachineEvent::Precommit(vote)]);
         output.append(&mut self.advance_to_step(Step::Precommit));
         output
@@ -518,7 +547,7 @@ impl StateMachine {
         } else {
             None
         };
-        let vote = self.make_self_vote(VoteType::Prevote, self.round, vote_commitment);
+        let vote = self.make_self_vote(VoteType::Prevote, vote_commitment);
         let mut output = VecDeque::from([StateMachineEvent::Prevote(vote)]);
         output.append(&mut self.advance_to_step(Step::Prevote));
         output
@@ -550,7 +579,7 @@ impl StateMachine {
         } else {
             None
         };
-        let vote = self.make_self_vote(VoteType::Prevote, self.round, vote_commitment);
+        let vote = self.make_self_vote(VoteType::Prevote, vote_commitment);
         let mut output = VecDeque::from([StateMachineEvent::Prevote(vote)]);
         output.append(&mut self.advance_to_step(Step::Prevote));
         output
@@ -600,7 +629,7 @@ impl StateMachine {
             CONSENSUS_NEW_VALUE_LOCKS.increment(1);
         }
         self.locked_value_round = new_value;
-        let vote = self.make_self_vote(VoteType::Precommit, self.round, Some(*proposal_id));
+        let vote = self.make_self_vote(VoteType::Precommit, Some(*proposal_id));
         let mut output = VecDeque::from([StateMachineEvent::Precommit(vote)]);
         output.append(&mut self.advance_to_step(Step::Precommit));
         output
@@ -614,7 +643,7 @@ impl StateMachine {
         if !self.value_has_enough_votes(&self.prevotes, self.round, &None, &self.quorum) {
             return VecDeque::new();
         }
-        let vote = self.make_self_vote(VoteType::Precommit, self.round, None);
+        let vote = self.make_self_vote(VoteType::Precommit, None);
         let mut output = VecDeque::from([StateMachineEvent::Precommit(vote)]);
         output.append(&mut self.advance_to_step(Step::Precommit));
         output

crates/apollo_protobuf/src/consensus.rs

@@ -37,7 +37,7 @@ impl<T> IntoFromProto for T where
 {
 }
 
-#[derive(Debug, Default, Hash, Clone, Eq, PartialEq, Serialize, Deserialize)]
+#[derive(Debug, Default, Hash, Clone, Copy, Eq, PartialEq, Serialize, Deserialize)]
 pub enum VoteType {
     Prevote,
     #[default]