Double Voting Prevention

crates/types/src/consensus.rs

@@ -322,6 +322,9 @@ pub struct Consensus<TYPES: NodeType> {
 
     /// Number of blocks in an epoch, zero means there are no epochs
     pub epoch_height: u64,
+
+    /// Vote tracker to prevent double voting
+    vote_tracker: VoteTracker<TYPES>,
 }
 
 /// Contains several `ConsensusMetrics` that we're interested in from the consensus interfaces
@@ -430,6 +433,7 @@ impl<TYPES: NodeType> Consensus<TYPES> {
             high_qc,
             metrics,
             epoch_height,
+            vote_tracker: VoteTracker::new(),
         }
     }
 
@@ -549,27 +553,39 @@ impl<TYPES: NodeType> Consensus<TYPES> {
     ///
     /// Returns true if the action is for a newer view than the last action of that type
     pub fn update_action(&mut self, action: HotShotAction, view: TYPES::View) -> bool {
-        let old_view = match action {
-            HotShotAction::Vote => &mut self.last_actions.voted,
-            HotShotAction::Propose => &mut self.last_actions.proposed,
-            HotShotAction::DaPropose => &mut self.last_actions.da_proposed,
+        match action {
             HotShotAction::DaVote => {
+                // Use vote tracker to prevent double voting
+                let voter_key = Arc::new(self.public_key().clone());
+
+                if !self.vote_tracker.record_vote(view, voter_key) {
+                    tracing::warn!("Prevented double voting attempt for view {}", view);
+                    return false;
+                }
+
                 if view > self.last_actions.da_vote {
                     self.last_actions.da_vote = view;
                 }
-                // TODO Add logic to prevent double voting.  For now the simple check if
-                // the last voted view is less than the view we are trying to vote doesn't work
-                // because the leader of view n + 1 may propose to the DA (and we would vote)
-                // before the leader of view n.
-                return true;
+
+                // Clean up old vote records periodically
+                self.vote_tracker.cleanup_old_views(view);
+
+                true
             }
+            HotShotAction::Vote => &mut self.last_actions.voted,
+            HotShotAction::Propose => &mut self.last_actions.proposed,
+            HotShotAction::DaPropose => &mut self.last_actions.da_proposed,
             _ => return true,
-        };
-        if view > *old_view {
-            *old_view = view;
-            return true;
         }
-        false
+        .map(|old_view| {
+            if view > *old_view {
+                *old_view = view;
+                true
+            } else {
+                false
+            }
+        })
+        .unwrap_or(true)
     }
 
     /// reset last actions to genesis so we can resend events in tests
@@ -1042,21 +1058,3 @@ impl<TYPES: NodeType> Consensus<TYPES> {
         new_epoch - 1 == old_epoch && self.is_leaf_extended(parent_leaf.commit())
     }
 }
-
-/// Alias for the block payload commitment and the associated metadata. The primary data
-/// needed in order to submit a proposal.
-#[derive(Eq, Hash, PartialEq, Debug, Clone)]
-pub struct CommitmentAndMetadata<TYPES: NodeType> {
-    /// Vid Commitment
-    pub commitment: VidCommitment,
-    /// Builder Commitment
-    pub builder_commitment: BuilderCommitment,
-    /// Metadata for the block payload
-    pub metadata: <TYPES::BlockPayload as BlockPayload<TYPES>>::Metadata,
-    /// Builder fee data
-    pub fees: Vec1<BuilderFee<TYPES>>,
-    /// View number this block is for
-    pub block_view: TYPES::View,
-    /// auction result that the block was produced from, if any
-    pub auction_result: Option<TYPES::AuctionResult>,
-}

crates/types/src/traits/vote_tracker.rs

@@ -0,0 +1,110 @@
+// Copyright (c) 2021-2024 Espresso Systems (espressosys.com)
+// This file is part of the HotShot repository.
+
+// You should have received a copy of the MIT License
+// along with the HotShot repository. If not, see <https://mit-license.org/>.
+
+//! Vote tracking implementation for preventing double voting
+
+use std::collections::{HashMap, HashSet};
+use std::sync::Arc;
+
+use crate::traits::node_implementation::NodeType;
+use crate::traits::signature_key::SignatureKey;
+
+/// Tracks votes to prevent double voting in HotShot consensus
+#[derive(Debug, Default)]
+pub struct VoteTracker<TYPES: NodeType> {
+    /// Maps view numbers to sets of voter public keys
+    view_votes: HashMap<TYPES::View, HashSet<Arc<TYPES::SignatureKey>>>,
+}
+
+impl<TYPES: NodeType> VoteTracker<TYPES> {
+    /// Creates a new VoteTracker instance
+    pub fn new() -> Self {
+        Self {
+            view_votes: HashMap::new(),
+        }
+    }
+
+    /// Records a vote for a specific view and voter
+    /// Returns true if the vote was successfully recorded (no double voting)
+    /// Returns false if this would be a double vote
+    pub fn record_vote(
+        &mut self,
+        view: TYPES::View,
+        voter_key: Arc<TYPES::SignatureKey>,
+    ) -> bool {
+        // Get or create the vote set for this view
+        let votes = self.view_votes.entry(view).or_default();
+
+        // Check if this voter has already voted
+        if votes.contains(&voter_key) {
+            return false;
+        }
+
+        // Record the vote
+        votes.insert(voter_key);
+        true
+    }
+
+    /// Cleans up old vote records for views that are no longer needed
+    /// This should be called periodically to prevent memory growth
+    pub fn cleanup_old_views(&mut self, current_view: TYPES::View) {
+        self.view_votes.retain(|view, _| *view >= current_view);
+    }
+
+    /// Returns true if the given voter has already voted in the specified view
+    pub fn has_voted(
+        &self,
+        view: &TYPES::View,
+        voter_key: &Arc<TYPES::SignatureKey>,
+    ) -> bool {
+        self.view_votes
+            .get(view)
+            .map_or(false, |votes| votes.contains(voter_key))
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::simple_vote::SimpleVote;
+    use std::sync::Arc;
+
+    #[test]
+    fn test_vote_tracking() {
+        let mut tracker = VoteTracker::<SimpleVote>::new();
+        let view = SimpleVote::View::default();
+        let key1 = Arc::new(SimpleVote::SignatureKey::default());
+        let key2 = Arc::new(SimpleVote::SignatureKey::default());
+
+        // First vote should succeed
+        assert!(tracker.record_vote(view, key1.clone()));
+
+        // Double vote should fail
+        assert!(!tracker.record_vote(view, key1.clone()));
+
+        // Different voter should succeed
+        assert!(tracker.record_vote(view, key2.clone()));
+    }
+
+    #[test]
+    fn test_cleanup() {
+        let mut tracker = VoteTracker::<SimpleVote>::new();
+        let view1 = SimpleVote::View::default();
+        let view2 = SimpleVote::View::default() + SimpleVote::View::from(1);
+        let key = Arc::new(SimpleVote::SignatureKey::default());
+
+        tracker.record_vote(view1, key.clone());
+        tracker.record_vote(view2, key.clone());
+
+        // Cleanup views before view2
+        tracker.cleanup_old_views(view2);
+
+        // Old vote should be removed
+        assert!(!tracker.has_voted(&view1, &key));
+        // Recent vote should remain
+        assert!(tracker.has_voted(&view2, &key));
+    }
+}