From 8dab3449720738d22ad51f6de8203af5394fa785 Mon Sep 17 00:00:00 2001
From: hanako mumei <81144685+2501babe@users.noreply.github.com>
Date: Thu, 18 Jul 2024 03:32:39 -0700
Subject: [PATCH] address feedback

---
 sdk/program/src/vote/state/mod.rs             | 33 ++++--------
 .../src/vote/state/vote_state_deserialize.rs  | 52 +++----------------
 2 files changed, 17 insertions(+), 68 deletions(-)

diff --git a/sdk/program/src/vote/state/mod.rs b/sdk/program/src/vote/state/mod.rs
index 9d048f46bdae35..08b19ec3a42af6 100644
--- a/sdk/program/src/vote/state/mod.rs
+++ b/sdk/program/src/vote/state/mod.rs
@@ -323,6 +323,7 @@ const MAX_ITEMS: usize = 32;
 
 #[cfg_attr(feature = "frozen-abi", derive(AbiExample))]
 #[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone)]
+#[cfg_attr(test, derive(Arbitrary))]
 pub struct CircBuf<I> {
     buf: [I; MAX_ITEMS],
     /// next pointer
@@ -368,23 +369,6 @@ impl<I> CircBuf<I> {
     }
 }
 
-#[cfg(test)]
-impl<'a, I: Default + Copy> Arbitrary<'a> for CircBuf<I>
-where
-    I: Arbitrary<'a>,
-{
-    fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<Self> {
-        let mut circbuf = Self::default();
-
-        let len = u.arbitrary_len::<I>()?;
-        for _ in 0..len {
-            circbuf.append(I::arbitrary(u)?);
-        }
-
-        Ok(circbuf)
-    }
-}
-
 #[cfg_attr(
     feature = "frozen-abi",
     frozen_abi(digest = "EeenjJaSrm9hRM39gK6raRNtzG61hnk7GciUCJJRDUSQ"),
@@ -528,12 +512,6 @@ impl VoteState {
             _ => Err(InstructionError::InvalidAccountData),
         }?;
 
-        // if cursor overruns the input, it produces null bytes and continues to advance `position`
-        // this check ensures we do not accept such a malformed input erroneously
-        if cursor.position() > input.len() as u64 {
-            return Err(InstructionError::InvalidAccountData);
-        }
-
         Ok(())
     }
 
@@ -1177,7 +1155,14 @@ mod tests {
             let test_res = VoteState::deserialize_into(&raw_data, &mut test_vote_state);
             let bincode_res = bincode::deserialize::<VoteStateVersions>(&raw_data);
 
-            assert_eq!(test_res.is_ok(), bincode_res.is_ok());
+            if test_res.is_err() {
+                assert!(bincode_res.is_err());
+            } else {
+                assert_eq!(
+                    VoteStateVersions::new_current(test_vote_state),
+                    bincode_res.unwrap()
+                );
+            }
         }
     }
 
diff --git a/sdk/program/src/vote/state/vote_state_deserialize.rs b/sdk/program/src/vote/state/vote_state_deserialize.rs
index f4628332c2f2c3..69fdf0636d9b57 100644
--- a/sdk/program/src/vote/state/vote_state_deserialize.rs
+++ b/sdk/program/src/vote/state/vote_state_deserialize.rs
@@ -7,10 +7,6 @@ use {
     std::io::Cursor,
 };
 
-// hardcode this number to avoid calculating onchain; this is a fixed-size ringbuffer
-// `serialized_size()` must be used over `mem::size_of()` because of alignment
-const PRIOR_VOTERS_SERIALIZED_SIZE: u64 = 1545;
-
 pub(super) fn deserialize_vote_state_into(
     cursor: &mut Cursor<&[u8]>,
     vote_state: &mut VoteState,
@@ -69,35 +65,17 @@ fn read_prior_voters_into<T: AsRef<[u8]>>(
     cursor: &mut Cursor<T>,
     vote_state: &mut VoteState,
 ) -> Result<(), InstructionError> {
-    // record our position at the start of the struct
-    let prior_voters_position = cursor.position();
-
-    let is_empty_position = PRIOR_VOTERS_SERIALIZED_SIZE
-        .checked_add(prior_voters_position)
-        .and_then(|v| v.checked_sub(1))
-        .ok_or(InstructionError::InvalidAccountData)?;
-
-    // move to the end, to check if we need to parse the data
-    cursor.set_position(is_empty_position);
-
-    // if empty, we already read past the end of this struct and need to do no further work
-    // otherwise we go back to the start and proceed to decode the data
-    let is_empty = read_bool(cursor)?;
-    if !is_empty {
-        cursor.set_position(prior_voters_position);
-
-        for i in 0..MAX_ITEMS {
-            let prior_voter = read_pubkey(cursor)?;
-            let from_epoch = read_u64(cursor)?;
-            let until_epoch = read_u64(cursor)?;
+    for i in 0..MAX_ITEMS {
+        let prior_voter = read_pubkey(cursor)?;
+        let from_epoch = read_u64(cursor)?;
+        let until_epoch = read_u64(cursor)?;
 
-            vote_state.prior_voters.buf[i] = (prior_voter, from_epoch, until_epoch);
-        }
-
-        vote_state.prior_voters.idx = read_u64(cursor)? as usize;
-        vote_state.prior_voters.is_empty = read_bool(cursor)?;
+        vote_state.prior_voters.buf[i] = (prior_voter, from_epoch, until_epoch);
     }
 
+    vote_state.prior_voters.idx = read_u64(cursor)? as usize;
+    vote_state.prior_voters.is_empty = read_bool(cursor)?;
+
     Ok(())
 }
 
@@ -131,17 +109,3 @@ fn read_last_timestamp_into<T: AsRef<[u8]>>(
 
     Ok(())
 }
-
-#[cfg(test)]
-mod tests {
-    use {super::*, bincode::serialized_size};
-
-    #[test]
-    fn test_prior_voters_serialized_size() {
-        let vote_state = VoteState::default();
-        assert_eq!(
-            serialized_size(&vote_state.prior_voters).unwrap(),
-            PRIOR_VOTERS_SERIALIZED_SIZE
-        );
-    }
-}