fix(code/core): Remove Ord constraint from Vote trait (#1148)

* remove unnecessary has_vote

* remove unnecessary restore_votes

* remove Ord trait from Vote and use Vec instead of BTreeSet

* set an initial capacity to avoid re-allocations

* Post-merge commit

* Restore check for seen votes

---------

Co-authored-by: Romain Ruetschi <[email protected]>

Author: insumity

code/crates/core-consensus/src/handle/vote.rs

@@ -76,19 +76,18 @@ where
 
     debug_assert_eq!(consensus_height, vote_height);
 
-    // Only append to WAL and store the non-nil precommit if we have not yet seen this vote.
+    // Only process this vote if we have not yet seen it.
     if !state.driver.votes().has_vote(&signed_vote) {
-        // Append the vote to the Write-ahead Log
         perform!(
             co,
             Effect::WalAppend(
                 WalEntry::ConsensusMsg(SignedConsensusMsg::Vote(signed_vote.clone())),
                 Default::default()
             )
         );
-    }
 
-    apply_driver_input(co, state, metrics, DriverInput::Vote(signed_vote)).await?;
+        apply_driver_input(co, state, metrics, DriverInput::Vote(signed_vote)).await?;
+    }
 
     Ok(())
 }

code/crates/core-consensus/src/state.rs

@@ -111,38 +111,6 @@ where
         }
     }
 
-    #[allow(clippy::type_complexity)]
-    pub fn restore_votes(
-        &mut self,
-        height: Ctx::Height,
-        round: Round,
-    ) -> Option<(Vec<SignedVote<Ctx>>, Vec<PolkaCertificate<Ctx>>)> {
-        assert!(round.is_defined());
-
-        if height != self.driver.height() {
-            return None;
-        }
-
-        let mut votes = Vec::new();
-
-        let upper_round = self.driver.votes().max_round();
-        for r in round_range_inclusive(round, upper_round) {
-            let per_round = self.driver.votes().per_round(r)?;
-            votes.extend(per_round.received_votes().iter().cloned());
-        }
-
-        // Gather polka certificates for all rounds up to `round` included
-        let certificates = self
-            .driver
-            .polka_certificates()
-            .iter()
-            .filter(|c| c.round <= round && c.height == height)
-            .cloned()
-            .collect::<Vec<_>>();
-
-        Some((votes, certificates))
-    }
-
     pub fn polka_certificate_at_round(&self, round: Round) -> Option<PolkaCertificate<Ctx>> {
         // Get the polka certificate for the specified round if it exists
         self.driver
@@ -274,15 +242,3 @@ where
         self.driver.round_certificate.as_ref()
     }
 }
-
-fn round_range_inclusive(from: Round, to: Round) -> Box<dyn Iterator<Item = Round>> {
-    if !from.is_defined() || !to.is_defined() || from > to {
-        return Box::new(std::iter::empty());
-    }
-
-    if from == to {
-        return Box::new(std::iter::once(from));
-    }
-
-    Box::new((from.as_u32().unwrap_or(0)..=to.as_u32().unwrap_or(0)).map(Round::new))
-}

code/crates/core-types/src/vote.rs

@@ -23,7 +23,7 @@ pub enum VoteType {
 /// include information about the validator signing it.
 pub trait Vote<Ctx>
 where
-    Self: Clone + Debug + Eq + Ord + Send + Sync + 'static,
+    Self: Clone + Debug + Eq + Send + Sync + 'static,
     Ctx: Context,
 {
     /// The height for which the vote is for.

code/crates/core-votekeeper/src/keeper.rs

@@ -4,7 +4,7 @@ use derive_where::derive_where;
 use thiserror::Error;
 
 use alloc::collections::{BTreeMap, BTreeSet};
-
+use alloc::vec::Vec;
 use malachitebft_core_types::{
     Context, NilOrVal, Round, SignedVote, Validator, ValidatorSet, ValueId, Vote, VoteType,
 };
@@ -49,7 +49,7 @@ where
     addresses_weights: RoundWeights<Ctx::Address>,
 
     /// All the votes received for this round.
-    received_votes: BTreeSet<SignedVote<Ctx>>,
+    received_votes: Vec<SignedVote<Ctx>>,
 
     /// The emitted outputs for this round.
     emitted_outputs: BTreeSet<Output<ValueId<Ctx>>>,
@@ -80,6 +80,15 @@ where
         Self::default()
     }
 
+    /// Create a new `PerRound` instance with pre-allocated capacity for the expected number of votes.
+    pub fn with_expected_number_of_votes(num_votes: usize) -> Self {
+        Self {
+            // pre-allocate capacity to avoid re-allocations during the addition of votes
+            received_votes: Vec::with_capacity(num_votes),
+            ..Self::default()
+        }
+    }
+
     /// Add a vote to the round, checking for conflicts.
     pub fn add(
         &mut self,
@@ -104,7 +113,7 @@ where
             .set_once(vote.validator_address(), weight);
 
         // Add the vote to the received votes
-        self.received_votes.insert(vote);
+        self.received_votes.push(vote);
 
         Ok(())
     }
@@ -126,7 +135,7 @@ where
     }
 
     /// Return the votes for this round.
-    pub fn received_votes(&self) -> &BTreeSet<SignedVote<Ctx>> {
+    pub fn received_votes(&self) -> &Vec<SignedVote<Ctx>> {
         &self.received_votes
     }
 
@@ -219,7 +228,12 @@ where
         round: Round,
     ) -> Option<Output<ValueId<Ctx>>> {
         let total_weight = self.total_weight();
-        let per_round = self.per_round.entry(vote.round()).or_default();
+        let per_round =
+            self.per_round
+                .entry(vote.round())
+                .or_insert(PerRound::with_expected_number_of_votes(
+                    self.validator_set.count(),
+                ));
 
         let Some(validator) = self.validator_set.get_by_address(vote.validator_address()) else {
             // Vote from unknown validator, let's discard it.