From 0c2d9d25fdd8dab9b2db29f6b7922f10c8e8b757 Mon Sep 17 00:00:00 2001 From: hana <81144685+2501babe@users.noreply.github.com> Date: Thu, 25 Jan 2024 08:51:41 -0800 Subject: [PATCH] solana-program: VoteState::deserialize() (#34829) * implement a custom parser for `VoteState` which is usuable in a bpf context * derive or impl `Arbitrary` for `VoteStateVersions` and its component types, for test builds only --- Cargo.lock | 21 +++ Cargo.toml | 1 + sdk/program/Cargo.toml | 1 + sdk/program/src/pubkey.rs | 4 + sdk/program/src/serialize_utils/cursor.rs | 133 ++++++++++++++++++ .../mod.rs} | 2 + sdk/program/src/vote/authorized_voters.rs | 3 + sdk/program/src/vote/state/mod.rs | 120 +++++++++++++++- .../src/vote/state/vote_state_1_14_11.rs | 3 + .../src/vote/state/vote_state_deserialize.rs | 129 +++++++++++++++++ .../src/vote/state/vote_state_versions.rs | 14 ++ 11 files changed, 424 insertions(+), 7 deletions(-) create mode 100644 sdk/program/src/serialize_utils/cursor.rs rename sdk/program/src/{serialize_utils.rs => serialize_utils/mod.rs} (99%) create mode 100644 sdk/program/src/vote/state/vote_state_deserialize.rs diff --git a/Cargo.lock b/Cargo.lock index 5378580c3c..d7c2a2405e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -181,6 +181,15 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "arbitrary" +version = "1.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7d5a26814d8dcb93b0e5a0ff3c6d80a8843bafb21b39e8e18a6f05471870e110" +dependencies = [ + "derive_arbitrary", +] + [[package]] name = "arc-swap" version = "1.5.0" @@ -1612,6 +1621,17 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "derive_arbitrary" +version = "1.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "67e77553c4162a157adbf834ebae5b415acbecbeafc7a74b0e886657506a7611" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.48", +] + [[package]] name = "derive_more" version = "0.99.16" @@ -6663,6 +6683,7 @@ name = "solana-program" version = "1.18.0" dependencies = [ "anyhow", + "arbitrary", "ark-bn254", "ark-ec", "ark-ff", diff --git a/Cargo.toml b/Cargo.toml index 71ab138fd9..242dfa13d0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -139,6 +139,7 @@ aquamarine = "0.3.3" aes-gcm-siv = "0.10.3" ahash = "0.8.7" anyhow = "1.0.79" +arbitrary = "1.3.2" ark-bn254 = "0.4.0" ark-ec = "0.4.0" ark-ff = "0.4.0" diff --git a/sdk/program/Cargo.toml b/sdk/program/Cargo.toml index ccd18701ee..7bc414472f 100644 --- a/sdk/program/Cargo.toml +++ b/sdk/program/Cargo.toml @@ -66,6 +66,7 @@ wasm-bindgen = { workspace = true } zeroize = { workspace = true, features = ["default", "zeroize_derive"] } [target.'cfg(not(target_os = "solana"))'.dev-dependencies] +arbitrary = { workspace = true, features = ["derive"] } solana-logger = { workspace = true } [target.'cfg(target_arch = "wasm32")'.dependencies] diff --git a/sdk/program/src/pubkey.rs b/sdk/program/src/pubkey.rs index 04fcc69dc9..728a5cd252 100644 --- a/sdk/program/src/pubkey.rs +++ b/sdk/program/src/pubkey.rs @@ -1,6 +1,9 @@ //! Solana account addresses. #![allow(clippy::arithmetic_side_effects)] + +#[cfg(test)] +use arbitrary::Arbitrary; use { crate::{decode_error::DecodeError, hash::hashv, wasm_bindgen}, borsh::{BorshDeserialize, BorshSchema, BorshSerialize}, @@ -85,6 +88,7 @@ impl From for PubkeyError { Zeroable, )] #[borsh(crate = "borsh")] +#[cfg_attr(test, derive(Arbitrary))] pub struct Pubkey(pub(crate) [u8; 32]); impl crate::sanitize::Sanitize for Pubkey {} diff --git a/sdk/program/src/serialize_utils/cursor.rs b/sdk/program/src/serialize_utils/cursor.rs new file mode 100644 index 0000000000..0066737382 --- /dev/null +++ b/sdk/program/src/serialize_utils/cursor.rs @@ -0,0 +1,133 @@ +use { + crate::{instruction::InstructionError, pubkey::Pubkey}, + std::io::{Cursor, Read}, +}; + +pub(crate) fn read_u8>(cursor: &mut Cursor) -> Result { + let mut buf = [0; 1]; + cursor + .read_exact(&mut buf) + .map_err(|_| InstructionError::InvalidAccountData)?; + + Ok(buf[0]) +} + +pub(crate) fn read_u32>(cursor: &mut Cursor) -> Result { + let mut buf = [0; 4]; + cursor + .read_exact(&mut buf) + .map_err(|_| InstructionError::InvalidAccountData)?; + + Ok(u32::from_le_bytes(buf)) +} + +pub(crate) fn read_u64>(cursor: &mut Cursor) -> Result { + let mut buf = [0; 8]; + cursor + .read_exact(&mut buf) + .map_err(|_| InstructionError::InvalidAccountData)?; + + Ok(u64::from_le_bytes(buf)) +} + +pub(crate) fn read_option_u64>( + cursor: &mut Cursor, +) -> Result, InstructionError> { + let variant = read_u8(cursor)?; + match variant { + 0 => Ok(None), + 1 => read_u64(cursor).map(Some), + _ => Err(InstructionError::InvalidAccountData), + } +} + +pub(crate) fn read_i64>(cursor: &mut Cursor) -> Result { + let mut buf = [0; 8]; + cursor + .read_exact(&mut buf) + .map_err(|_| InstructionError::InvalidAccountData)?; + + Ok(i64::from_le_bytes(buf)) +} + +pub(crate) fn read_pubkey>( + cursor: &mut Cursor, +) -> Result { + let mut buf = [0; 32]; + cursor + .read_exact(&mut buf) + .map_err(|_| InstructionError::InvalidAccountData)?; + + Ok(Pubkey::from(buf)) +} + +#[cfg(test)] +mod test { + use {super::*, rand::Rng, std::fmt::Debug}; + + #[test] + fn test_read_u8() { + for _ in 0..100 { + let test_value = rand::random::(); + test_read(read_u8, test_value); + } + } + + #[test] + fn test_read_u32() { + for _ in 0..100 { + let test_value = rand::random::(); + test_read(read_u32, test_value); + } + } + + #[test] + fn test_read_u64() { + for _ in 0..100 { + let test_value = rand::random::(); + test_read(read_u64, test_value); + } + } + + #[test] + fn test_read_option_u64() { + for _ in 0..100 { + let test_value = rand::random::>(); + test_read(read_option_u64, test_value); + } + } + + #[test] + fn test_read_i64() { + for _ in 0..100 { + let test_value = rand::random::(); + test_read(read_i64, test_value); + } + } + + #[test] + fn test_read_pubkey() { + for _ in 0..100 { + let mut buf = [0; 32]; + rand::thread_rng().fill(&mut buf); + let test_value = Pubkey::from(buf); + test_read(read_pubkey, test_value); + } + } + + fn test_read( + reader: fn(&mut Cursor>) -> Result, + test_value: T, + ) { + let bincode_bytes = bincode::serialize(&test_value).unwrap(); + let mut cursor = Cursor::new(bincode_bytes); + let bincode_read = reader(&mut cursor).unwrap(); + + let borsh_bytes = borsh0_10::to_vec(&test_value).unwrap(); + let mut cursor = Cursor::new(borsh_bytes); + let borsh_read = reader(&mut cursor).unwrap(); + + assert_eq!(test_value, bincode_read); + assert_eq!(test_value, borsh_read); + } +} diff --git a/sdk/program/src/serialize_utils.rs b/sdk/program/src/serialize_utils/mod.rs similarity index 99% rename from sdk/program/src/serialize_utils.rs rename to sdk/program/src/serialize_utils/mod.rs index d57095ce7a..1e335483f9 100644 --- a/sdk/program/src/serialize_utils.rs +++ b/sdk/program/src/serialize_utils/mod.rs @@ -3,6 +3,8 @@ #![allow(clippy::arithmetic_side_effects)] use crate::{pubkey::Pubkey, sanitize::SanitizeError}; +pub mod cursor; + pub fn append_u16(buf: &mut Vec, data: u16) { let start = buf.len(); buf.resize(buf.len() + 2, 0); diff --git a/sdk/program/src/vote/authorized_voters.rs b/sdk/program/src/vote/authorized_voters.rs index f361be237d..9920391146 100644 --- a/sdk/program/src/vote/authorized_voters.rs +++ b/sdk/program/src/vote/authorized_voters.rs @@ -1,3 +1,5 @@ +#[cfg(test)] +use arbitrary::Arbitrary; use { crate::{clock::Epoch, pubkey::Pubkey}, serde_derive::{Deserialize, Serialize}, @@ -5,6 +7,7 @@ use { }; #[derive(Debug, Default, Serialize, Deserialize, PartialEq, Eq, Clone, AbiExample)] +#[cfg_attr(test, derive(Arbitrary))] pub struct AuthorizedVoters { authorized_voters: BTreeMap, } diff --git a/sdk/program/src/vote/state/mod.rs b/sdk/program/src/vote/state/mod.rs index 6d77d3ab5d..9eddce4d94 100644 --- a/sdk/program/src/vote/state/mod.rs +++ b/sdk/program/src/vote/state/mod.rs @@ -1,9 +1,12 @@ //! Vote state -#[cfg(test)] -use crate::epoch_schedule::MAX_LEADER_SCHEDULE_EPOCH_OFFSET; #[cfg(not(target_os = "solana"))] use bincode::deserialize; +#[cfg(test)] +use { + crate::epoch_schedule::MAX_LEADER_SCHEDULE_EPOCH_OFFSET, + arbitrary::{Arbitrary, Unstructured}, +}; use { crate::{ clock::{Epoch, Slot, UnixTimestamp}, @@ -11,17 +14,20 @@ use { instruction::InstructionError, pubkey::Pubkey, rent::Rent, + serialize_utils::cursor::read_u32, sysvar::clock::Clock, vote::{authorized_voters::AuthorizedVoters, error::VoteError}, }, bincode::{serialize_into, ErrorKind}, serde_derive::{Deserialize, Serialize}, - std::{collections::VecDeque, fmt::Debug}, + std::{collections::VecDeque, fmt::Debug, io::Cursor}, }; mod vote_state_0_23_5; pub mod vote_state_1_14_11; pub use vote_state_1_14_11::*; +mod vote_state_deserialize; +use vote_state_deserialize::deserialize_vote_state_into; pub mod vote_state_versions; pub use vote_state_versions::*; @@ -67,6 +73,7 @@ impl Vote { } #[derive(Serialize, Default, Deserialize, Debug, PartialEq, Eq, Copy, Clone, AbiExample)] +#[cfg_attr(test, derive(Arbitrary))] pub struct Lockout { slot: Slot, confirmation_count: u32, @@ -114,6 +121,7 @@ impl Lockout { } #[derive(Serialize, Default, Deserialize, Debug, PartialEq, Eq, Copy, Clone, AbiExample)] +#[cfg_attr(test, derive(Arbitrary))] pub struct LandedVote { // Latency is the difference in slot number between the slot that was voted on (lockout.slot) and the slot in // which the vote that added this Lockout landed. For votes which were cast before versions of the validator @@ -226,6 +234,7 @@ pub struct VoteAuthorizeCheckedWithSeedArgs { } #[derive(Debug, Default, Serialize, Deserialize, PartialEq, Eq, Clone, AbiExample)] +#[cfg_attr(test, derive(Arbitrary))] pub struct BlockTimestamp { pub slot: Slot, pub timestamp: UnixTimestamp, @@ -280,8 +289,26 @@ impl CircBuf { } } +#[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) + } +} + #[frozen_abi(digest = "EeenjJaSrm9hRM39gK6raRNtzG61hnk7GciUCJJRDUSQ")] #[derive(Debug, Default, Serialize, Deserialize, PartialEq, Eq, Clone, AbiExample)] +#[cfg_attr(test, derive(Arbitrary))] pub struct VoteState { /// the node that votes in this account pub node_pubkey: Pubkey, @@ -347,16 +374,43 @@ impl VoteState { 3762 // see test_vote_state_size_of. } - #[allow(clippy::used_underscore_binding)] - pub fn deserialize(_input: &[u8]) -> Result { + // we retain bincode deserialize for not(target_os = "solana") + // because the hand-written parser does not support V0_23_5 + pub fn deserialize(input: &[u8]) -> Result { #[cfg(not(target_os = "solana"))] { - deserialize::(_input) + deserialize::(input) .map(|versioned| versioned.convert_to_current()) .map_err(|_| InstructionError::InvalidAccountData) } #[cfg(target_os = "solana")] - unimplemented!(); + { + let mut vote_state = Self::default(); + Self::deserialize_into(input, &mut vote_state)?; + Ok(vote_state) + } + } + + /// Deserializes the input buffer into the provided `VoteState` + /// + /// 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`. + pub fn deserialize_into( + input: &[u8], + vote_state: &mut VoteState, + ) -> Result<(), InstructionError> { + 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), + // 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 + 2 => deserialize_vote_state_into(&mut cursor, vote_state, true), + _ => Err(InstructionError::InvalidAccountData), + } } pub fn serialize( @@ -818,6 +872,58 @@ mod tests { ); } + #[test] + fn test_vote_deserialize_into() { + // base case + let target_vote_state = VoteState::default(); + let vote_state_buf = + bincode::serialize(&VoteStateVersions::new_current(target_vote_state.clone())).unwrap(); + + 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); + + // 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..1000 { + let raw_data: Vec = (0..struct_bytes_x4).map(|_| rand::random::()).collect(); + let mut unstructured = Unstructured::new(&raw_data); + + let target_vote_state_versions = + VoteStateVersions::arbitrary(&mut unstructured).unwrap(); + 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); + } + } + + #[test] + fn test_vote_deserialize_into_nopanic() { + // base case + let mut test_vote_state = VoteState::default(); + let e = VoteState::deserialize_into(&[], &mut test_vote_state).unwrap_err(); + assert_eq!(e, InstructionError::InvalidAccountData); + + // variant + let serialized_len_x4 = bincode::serialized_size(&test_vote_state).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(); + + // 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 + let mut test_vote_state = VoteState::default(); + let _ = VoteState::deserialize_into(&raw_data, &mut test_vote_state); + } + } + #[test] fn test_vote_state_commission_split() { let vote_state = VoteState::default(); diff --git a/sdk/program/src/vote/state/vote_state_1_14_11.rs b/sdk/program/src/vote/state/vote_state_1_14_11.rs index 2e2f17484c..4b68ced365 100644 --- a/sdk/program/src/vote/state/vote_state_1_14_11.rs +++ b/sdk/program/src/vote/state/vote_state_1_14_11.rs @@ -1,10 +1,13 @@ use super::*; +#[cfg(test)] +use arbitrary::Arbitrary; // Offset used for VoteState version 1_14_11 const DEFAULT_PRIOR_VOTERS_OFFSET: usize = 82; #[frozen_abi(digest = "CZTgLymuevXjAx6tM8X8T5J3MCx9AkEsFSmu4FJrEpkG")] #[derive(Debug, Default, Serialize, Deserialize, PartialEq, Eq, Clone, AbiExample)] +#[cfg_attr(test, derive(Arbitrary))] pub struct VoteState1_14_11 { /// the node that votes in this account pub node_pubkey: Pubkey, diff --git a/sdk/program/src/vote/state/vote_state_deserialize.rs b/sdk/program/src/vote/state/vote_state_deserialize.rs new file mode 100644 index 0000000000..b93f1c7442 --- /dev/null +++ b/sdk/program/src/vote/state/vote_state_deserialize.rs @@ -0,0 +1,129 @@ +use { + crate::{ + instruction::InstructionError, + pubkey::Pubkey, + serialize_utils::cursor::*, + vote::state::{BlockTimestamp, LandedVote, Lockout, VoteState, MAX_ITEMS}, + }, + std::io::Cursor, +}; + +pub(super) fn deserialize_vote_state_into( + cursor: &mut Cursor<&[u8]>, + vote_state: &mut VoteState, + has_latency: bool, +) -> Result<(), InstructionError> { + vote_state.node_pubkey = read_pubkey(cursor)?; + vote_state.authorized_withdrawer = read_pubkey(cursor)?; + vote_state.commission = read_u8(cursor)?; + read_votes_into(cursor, vote_state, has_latency)?; + vote_state.root_slot = read_option_u64(cursor)?; + read_authorized_voters_into(cursor, vote_state)?; + read_prior_voters_into(cursor, vote_state)?; + read_epoch_credits_into(cursor, vote_state)?; + read_last_timestamp_into(cursor, vote_state)?; + + Ok(()) +} + +fn read_votes_into>( + cursor: &mut Cursor, + vote_state: &mut VoteState, + has_latency: bool, +) -> Result<(), InstructionError> { + let vote_count = read_u64(cursor)?; + + for _ in 0..vote_count { + let latency = if has_latency { read_u8(cursor)? } else { 0 }; + + let slot = read_u64(cursor)?; + let confirmation_count = read_u32(cursor)?; + let lockout = Lockout::new_with_confirmation_count(slot, confirmation_count); + + vote_state.votes.push_back(LandedVote { latency, lockout }); + } + + Ok(()) +} + +fn read_authorized_voters_into>( + cursor: &mut Cursor, + vote_state: &mut VoteState, +) -> Result<(), InstructionError> { + let authorized_voter_count = read_u64(cursor)?; + + for _ in 0..authorized_voter_count { + let epoch = read_u64(cursor)?; + let authorized_voter = read_pubkey(cursor)?; + + vote_state.authorized_voters.insert(epoch, authorized_voter); + } + + Ok(()) +} + +fn read_prior_voters_into>( + cursor: &mut Cursor, + vote_state: &mut VoteState, +) -> Result<(), InstructionError> { + 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; + } + } + + let idx = read_u64(cursor)? as usize; + vote_state.prior_voters.idx = idx; + + let is_empty_byte = read_u8(cursor)?; + let is_empty = match is_empty_byte { + 0 => false, + 1 => true, + _ => return Err(InstructionError::InvalidAccountData), + }; + vote_state.prior_voters.is_empty = is_empty; + + Ok(()) +} + +fn read_epoch_credits_into>( + cursor: &mut Cursor, + vote_state: &mut VoteState, +) -> Result<(), InstructionError> { + let epoch_credit_count = read_u64(cursor)?; + + for _ in 0..epoch_credit_count { + let epoch = read_u64(cursor)?; + let credits = read_u64(cursor)?; + let prev_credits = read_u64(cursor)?; + + vote_state + .epoch_credits + .push((epoch, credits, prev_credits)); + } + + Ok(()) +} + +fn read_last_timestamp_into>( + cursor: &mut Cursor, + vote_state: &mut VoteState, +) -> Result<(), InstructionError> { + let slot = read_u64(cursor)?; + let timestamp = read_i64(cursor)?; + + vote_state.last_timestamp = BlockTimestamp { slot, timestamp }; + + Ok(()) +} diff --git a/sdk/program/src/vote/state/vote_state_versions.rs b/sdk/program/src/vote/state/vote_state_versions.rs index 7c4939d369..58d63d15de 100644 --- a/sdk/program/src/vote/state/vote_state_versions.rs +++ b/sdk/program/src/vote/state/vote_state_versions.rs @@ -1,4 +1,6 @@ use super::{vote_state_0_23_5::VoteState0_23_5, vote_state_1_14_11::VoteState1_14_11, *}; +#[cfg(test)] +use arbitrary::{Arbitrary, Unstructured}; #[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone)] pub enum VoteStateVersions { @@ -90,3 +92,15 @@ impl VoteStateVersions { || VoteState1_14_11::is_correct_size_and_initialized(data) } } + +#[cfg(test)] +impl Arbitrary<'_> for VoteStateVersions { + fn arbitrary(u: &mut Unstructured<'_>) -> arbitrary::Result { + let variant = u.choose_index(2)?; + match variant { + 0 => Ok(Self::Current(Box::new(VoteState::arbitrary(u)?))), + 1 => Ok(Self::V1_14_11(Box::new(VoteState1_14_11::arbitrary(u)?))), + _ => unreachable!(), + } + } +}