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(),
                     ));
@@ -502,37 +498,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

@@ -90,6 +90,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 {
@@ -122,6 +125,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,
         }
     }
 
@@ -161,13 +166,40 @@ 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 vt = vote_type.clone();
+        let vote = Vote {
+            vote_type: vt,
+            height: self.height.0,
+            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,
+        };
+        if match last_self_vote {
+            None => true,
+            Some(last) => round > last.round,
+        } {
+            *last_self_vote = Some(vote.clone());
+        }
+        vote
     }
 
     /// Starts the state machine, effectively calling `StartRound(0)` from the paper. This is