diff --git a/sdk/program/src/vote/state/mod.rs b/sdk/program/src/vote/state/mod.rs index 1342fb9deea4a3..9d048f46bdae35 100644 --- a/sdk/program/src/vote/state/mod.rs +++ b/sdk/program/src/vote/state/mod.rs @@ -18,7 +18,7 @@ use { sysvar::clock::Clock, vote::{authorized_voters::AuthorizedVoters, error::VoteError}, }, - bincode::{serialize_into, serialized_size, ErrorKind}, + bincode::{serialize_into, ErrorKind}, serde_derive::{Deserialize, Serialize}, std::{collections::VecDeque, fmt::Debug, io::Cursor}, }; @@ -475,8 +475,11 @@ impl VoteState { 3762 // see test_vote_state_size_of. } - // we retain bincode deserialize for not(target_os = "solana") - // because the hand-written parser does not support V0_23_5 + // NOTE we retain `bincode::deserialize` for `not(target_os = "solana")` pending testing on mainnet-beta + // once that testing is done, `VoteState::deserialize_into` may be used for all targets + // conversion of V0_23_5 to current must be handled specially, however + // because it inserts a null voter into `authorized_voters` + // which `VoteStateVersions::is_uninitialized` erroneously reports as initialized pub fn deserialize(input: &[u8]) -> Result { #[cfg(not(target_os = "solana"))] { @@ -492,26 +495,32 @@ impl VoteState { } } - /// Deserializes the input buffer into the provided `VoteState` + /// Deserializes the input `VoteStateVersions` buffer directly into a provided `VoteState` struct /// - /// This function exists to deserialize `VoteState` in a BPF context without going above - /// the compute limit, and must be kept up to date with `bincode::deserialize`. + /// In a BPF context, V0_23_5 is not supported, but in non-BPF, all versions are supported for + /// compatibility with `bincode::deserialize` pub fn deserialize_into( input: &[u8], vote_state: &mut VoteState, ) -> Result<(), InstructionError> { - let minimum_size = - serialized_size(vote_state).map_err(|_| InstructionError::InvalidAccountData)?; - if (input.len() as u64) < minimum_size { - return Err(InstructionError::InvalidAccountData); - } - let mut cursor = Cursor::new(input); let variant = read_u32(&mut cursor)?; match variant { - // V0_23_5. not supported; these should not exist on mainnet - 0 => Err(InstructionError::InvalidAccountData), + // V0_23_5. not supported for bpf targets; these should not exist on mainnet + // supported for non-bpf targets for backwards compatibility + 0 => { + #[cfg(not(target_os = "solana"))] + { + *vote_state = bincode::deserialize::(input) + .map(|versioned| versioned.convert_to_current()) + .map_err(|_| InstructionError::InvalidAccountData)?; + + Ok(()) + } + #[cfg(target_os = "solana")] + Err(InstructionError::InvalidAccountData) + } // V1_14_11. substantially different layout and data from V0_23_5 1 => deserialize_vote_state_into(&mut cursor, vote_state, false), // Current. the only difference from V1_14_11 is the addition of a slot-latency to each vote @@ -519,6 +528,8 @@ 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); } @@ -1087,7 +1098,7 @@ pub mod serde_tower_sync { #[cfg(test)] mod tests { - use {super::*, itertools::Itertools, rand::Rng}; + use {super::*, bincode::serialized_size, itertools::Itertools, rand::Rng}; #[test] fn test_vote_serialize() { @@ -1145,16 +1156,28 @@ mod tests { assert_eq!(e, InstructionError::InvalidAccountData); // variant - let serialized_len_x4 = serialized_size(&test_vote_state).unwrap() * 4; + let serialized_len_x4 = serialized_size(&VoteState::default()).unwrap() * 4; let mut rng = rand::thread_rng(); for _ in 0..1000 { let raw_data_length = rng.gen_range(1..serialized_len_x4); - let raw_data: Vec = (0..raw_data_length).map(|_| rng.gen::()).collect(); + let mut raw_data: Vec = (0..raw_data_length).map(|_| rng.gen::()).collect(); + + // pure random data will ~never have a valid enum tag, so lets help it out + if raw_data_length >= 4 && rng.gen::() { + let tag = rng.gen::() % 3; + raw_data[0] = tag; + raw_data[1] = 0; + raw_data[2] = 0; + raw_data[3] = 0; + } // it is extremely improbable, though theoretically possible, for random bytes to be syntactically valid - // so we only check that the deserialize function does not panic + // so we only check that the parser does not panic and that it succeeds or fails exactly in line with bincode let mut test_vote_state = VoteState::default(); - let _ = VoteState::deserialize_into(&raw_data, &mut test_vote_state); + let test_res = VoteState::deserialize_into(&raw_data, &mut test_vote_state); + let bincode_res = bincode::deserialize::(&raw_data); + + assert_eq!(test_res.is_ok(), bincode_res.is_ok()); } } diff --git a/sdk/program/src/vote/state/vote_state_0_23_5.rs b/sdk/program/src/vote/state/vote_state_0_23_5.rs index ae3b9207fe494e..2537633690c7ea 100644 --- a/sdk/program/src/vote/state/vote_state_0_23_5.rs +++ b/sdk/program/src/vote/state/vote_state_0_23_5.rs @@ -1,9 +1,12 @@ #![allow(clippy::arithmetic_side_effects)] use super::*; +#[cfg(test)] +use arbitrary::{Arbitrary, Unstructured}; const MAX_ITEMS: usize = 32; #[derive(Debug, Default, Serialize, Deserialize, PartialEq, Eq, Clone)] +#[cfg_attr(test, derive(Arbitrary))] pub struct VoteState0_23_5 { /// the node that votes in this account pub node_pubkey: Pubkey, @@ -59,3 +62,61 @@ impl CircBuf { self.buf[self.idx] = item; } } + +#[cfg(test)] +impl<'a, I: Default + Copy> Arbitrary<'a> for CircBuf +where + I: Arbitrary<'a>, +{ + fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result { + let mut circbuf = Self::default(); + + let len = u.arbitrary_len::()?; + for _ in 0..len { + circbuf.append(I::arbitrary(u)?); + } + + Ok(circbuf) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_vote_deserialize_0_23_5() { + // base case + let target_vote_state = VoteState0_23_5::default(); + let target_vote_state_versions = VoteStateVersions::V0_23_5(Box::new(target_vote_state)); + let vote_state_buf = bincode::serialize(&target_vote_state_versions).unwrap(); + + let mut test_vote_state = VoteState::default(); + VoteState::deserialize_into(&vote_state_buf, &mut test_vote_state).unwrap(); + + assert_eq!( + target_vote_state_versions.convert_to_current(), + test_vote_state + ); + + // variant + // provide 4x the minimum struct size in bytes to ensure we typically touch every field + let struct_bytes_x4 = std::mem::size_of::() * 4; + for _ in 0..100 { + let raw_data: Vec = (0..struct_bytes_x4).map(|_| rand::random::()).collect(); + let mut unstructured = Unstructured::new(&raw_data); + + let arbitrary_vote_state = VoteState0_23_5::arbitrary(&mut unstructured).unwrap(); + let target_vote_state_versions = + VoteStateVersions::V0_23_5(Box::new(arbitrary_vote_state)); + + let vote_state_buf = bincode::serialize(&target_vote_state_versions).unwrap(); + let target_vote_state = target_vote_state_versions.convert_to_current(); + + let mut test_vote_state = VoteState::default(); + VoteState::deserialize_into(&vote_state_buf, &mut test_vote_state).unwrap(); + + assert_eq!(target_vote_state, test_vote_state); + } + } +} diff --git a/sdk/program/src/vote/state/vote_state_deserialize.rs b/sdk/program/src/vote/state/vote_state_deserialize.rs index b457395ccbd38a..f4628332c2f2c3 100644 --- a/sdk/program/src/vote/state/vote_state_deserialize.rs +++ b/sdk/program/src/vote/state/vote_state_deserialize.rs @@ -1,14 +1,16 @@ use { crate::{ instruction::InstructionError, - pubkey::Pubkey, serialize_utils::cursor::*, vote::state::{BlockTimestamp, LandedVote, Lockout, VoteState, MAX_ITEMS}, }, - bincode::serialized_size, 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, @@ -70,10 +72,8 @@ fn read_prior_voters_into>( // record our position at the start of the struct let prior_voters_position = cursor.position(); - // `serialized_size()` must be used over `mem::size_of()` because of alignment - let is_empty_position = serialized_size(&vote_state.prior_voters) - .ok() - .and_then(|v| v.checked_add(prior_voters_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)?; @@ -86,21 +86,12 @@ fn read_prior_voters_into>( if !is_empty { cursor.set_position(prior_voters_position); - let mut encountered_null_voter = false; for i in 0..MAX_ITEMS { let prior_voter = read_pubkey(cursor)?; let from_epoch = read_u64(cursor)?; let until_epoch = read_u64(cursor)?; - let item = (prior_voter, from_epoch, until_epoch); - - if item == (Pubkey::default(), 0, 0) { - encountered_null_voter = true; - } else if encountered_null_voter { - // `prior_voters` should never be sparse - return Err(InstructionError::InvalidAccountData); - } else { - vote_state.prior_voters.buf[i] = item; - } + + vote_state.prior_voters.buf[i] = (prior_voter, from_epoch, until_epoch); } vote_state.prior_voters.idx = read_u64(cursor)? as usize; @@ -140,3 +131,17 @@ fn read_last_timestamp_into>( 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 + ); + } +}