PR feedback

rs/nns/governance/src/governance.rs

@@ -2318,7 +2318,7 @@ impl Governance {
                     // we are using a circular buffer to store them.  This solution is not ideal, but
                     // we need to do a larger refactoring to use the correct API types instead of the internal
                     // governance proto at this level.
-                    proto.recent_ballots = neuron.recent_ballots();
+                    proto.recent_ballots = neuron.sorted_recent_ballots();
                     full_neurons.push(proto);
                 }
             });

rs/nns/governance/src/neuron/types.rs

@@ -77,6 +77,7 @@ pub struct Neuron {
     /// Map `Topic` to followees. The key is represented by an integer as
     /// Protobuf does not support enum keys in maps.
     pub followees: HashMap<i32, Followees>,
+    // TODO DO NOT MERGE make this private
     /// Information about how this neuron voted in the recent past. It
     /// only contains proposals that the neuron voted yes or no on.
     pub recent_ballots: Vec<BallotInfo>,
@@ -436,18 +437,18 @@ impl Neuron {
         std::cmp::min(ad_stake, u64::MAX as u128) as u64
     }
 
-    pub(crate) fn recent_ballots(&self) -> Vec<BallotInfo> {
-        if self.recent_ballots_next_entry_index.is_none() {
-            self.recent_ballots.clone()
-        } else {
+    /// Get the recent ballots, with most recent ballots first
+    pub(crate) fn sorted_recent_ballots(&self) -> Vec<BallotInfo> {
+        if let Some(index) = self.recent_ballots_next_entry_index {
             let recent_ballots = &self.recent_ballots;
-            let index = self.recent_ballots_next_entry_index.unwrap();
             recent_ballots[index..]
                 .iter()
                 .chain(recent_ballots[..index].iter())
                 .rev()
                 .cloned()
                 .collect()
+        } else {
+            self.recent_ballots.clone()
         }
     }
 
@@ -906,7 +907,7 @@ impl Neuron {
             || self.visibility() == Some(Visibility::Public)
             || self.is_hotkey_or_controller(&requester);
         if show_full {
-            recent_ballots.append(&mut self.recent_ballots());
+            recent_ballots.append(&mut self.sorted_recent_ballots());
             joined_community_fund_timestamp_seconds = self.joined_community_fund_timestamp_seconds;
         }
 

rs/nns/governance/src/neuron/types/tests.rs

@@ -748,7 +748,7 @@ fn test_recent_ballots_accessor_pre_and_post_migration() {
     .build();
     neuron.recent_ballots_next_entry_index = None;
 
-    assert_eq!(neuron.recent_ballots(), recent_ballots);
+    assert_eq!(neuron.sorted_recent_ballots(), recent_ballots);
 
     neuron.register_recent_ballot(Topic::NetworkEconomics, &ProposalId { id: 100 }, Vote::No);
     assert_eq!(neuron.recent_ballots_next_entry_index, Some(1));
@@ -766,5 +766,5 @@ fn test_recent_ballots_accessor_pre_and_post_migration() {
         recent_ballots
     };
 
-    assert_eq!(neuron.recent_ballots(), expected_updated_ballots);
+    assert_eq!(neuron.sorted_recent_ballots(), expected_updated_ballots);
 }