From ce116e8f41da10ef363215a48a5a22d90972e517 Mon Sep 17 00:00:00 2001
From: Max Summe <maximilian.summe@dfinity.org>
Date: Fri, 22 Nov 2024 12:58:16 -0800
Subject: [PATCH] PR feedback

---
 rs/nns/governance/src/governance.rs         |  2 +-
 rs/nns/governance/src/neuron/types.rs       | 13 +++++++------
 rs/nns/governance/src/neuron/types/tests.rs |  4 ++--
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/rs/nns/governance/src/governance.rs b/rs/nns/governance/src/governance.rs
index c0e7d7659587..b9b4a3122007 100644
--- a/rs/nns/governance/src/governance.rs
+++ b/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);
                 }
             });
diff --git a/rs/nns/governance/src/neuron/types.rs b/rs/nns/governance/src/neuron/types.rs
index d615147d522e..45cb6e936dec 100644
--- a/rs/nns/governance/src/neuron/types.rs
+++ b/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;
         }
 
diff --git a/rs/nns/governance/src/neuron/types/tests.rs b/rs/nns/governance/src/neuron/types/tests.rs
index e857f33d0aac..00371c694dae 100644
--- a/rs/nns/governance/src/neuron/types/tests.rs
+++ b/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);
 }