Refactor: move simple vote parsing to runtime (backport #22537) (#22587)

* Refactor: move simple vote parsing to runtime (#22537)

(cherry picked from commit 7f20c6149e)

# Conflicts:
#	core/src/cluster_info_vote_listener.rs
#	core/src/verified_vote_packets.rs
#	programs/vote/src/vote_transaction.rs
#	rpc/src/rpc_subscriptions.rs
#	runtime/src/bank.rs
#	runtime/src/bank_utils.rs
#	runtime/src/vote_sender_types.rs

* resolve conflicts

Co-authored-by: Justin Starry <justin@solana.com>

core/src/cluster_info_vote_listener.rs

@@ -32,6 +32,7 @@ use {
         bank_forks::BankForks,
         commitment::VOTE_THRESHOLD_SIZE,
         epoch_stakes::EpochStakes,
+        vote_parser,
         vote_sender_types::{ReplayVoteReceiver, ReplayedVote},
     },
     solana_sdk::{
@@ -42,7 +43,7 @@ use {
         slot_hashes,
         transaction::Transaction,
     },
-    solana_vote_program::{self, vote_state::Vote, vote_transaction},
+    solana_vote_program::vote_state::Vote,
     std::{
         collections::{HashMap, HashSet},
         iter::repeat,
@@ -311,7 +312,7 @@ impl ClusterInfoVoteListener {
                 !packet_batch.packets[0].meta.discard()
             })
             .filter_map(|(tx, packet_batch)| {
-                let (vote_account_key, vote, _) = vote_transaction::parse_vote_transaction(&tx)?;
+                let (vote_account_key, vote, _) = vote_parser::parse_vote_transaction(&tx)?;
                 let slot = vote.last_voted_slot()?;
                 let epoch = epoch_schedule.get_epoch(slot);
                 let authorized_voter = root_bank
@@ -705,7 +706,7 @@ impl ClusterInfoVoteListener {
         // Process votes from gossip and ReplayStage
         let votes = gossip_vote_txs
             .iter()
-            .filter_map(vote_transaction::parse_vote_transaction)
+            .filter_map(vote_parser::parse_vote_transaction)
             .zip(repeat(/*is_gossip:*/ true))
             .chain(replayed_votes.into_iter().zip(repeat(/*is_gossip:*/ false)));
         for ((vote_pubkey, vote, _), is_gossip) in votes {
@@ -823,7 +824,7 @@ mod tests {
             pubkey::Pubkey,
             signature::{Keypair, Signature, Signer},
         },
-        solana_vote_program::vote_state::Vote,
+        solana_vote_program::{vote_state::Vote, vote_transaction},
         std::{
             collections::BTreeSet,
             iter::repeat_with,

gossip/src/cluster_info.rs

@@ -52,7 +52,7 @@ use {
         },
     },
     solana_rayon_threadlimit::get_thread_count,
-    solana_runtime::bank_forks::BankForks,
+    solana_runtime::{bank_forks::BankForks, vote_parser},
     solana_sdk::{
         clock::{Slot, DEFAULT_MS_PER_SLOT, DEFAULT_SLOTS_PER_EPOCH},
         feature_set::FeatureSet,
@@ -69,9 +69,7 @@ use {
         socket::SocketAddrSpace,
         streamer::{PacketBatchReceiver, PacketBatchSender},
     },
-    solana_vote_program::{
+    solana_vote_program::vote_state::MAX_LOCKOUT_HISTORY,
-        vote_state::MAX_LOCKOUT_HISTORY, vote_transaction::parse_vote_transaction,
-    },
     std::{
         borrow::Cow,
         collections::{hash_map::Entry, HashMap, HashSet, VecDeque},
@@ -1037,7 +1035,7 @@ impl ClusterInfo {
         };
         let vote_index = vote_index.unwrap_or(num_crds_votes);
         if (vote_index as usize) >= MAX_LOCKOUT_HISTORY {
-            let (_, vote, hash) = parse_vote_transaction(&vote).unwrap();
+            let (_, vote, hash) = vote_parser::parse_vote_transaction(&vote).unwrap();
             panic!(
                 "invalid vote index: {}, switch: {}, vote slots: {:?}, tower: {:?}",
                 vote_index,

gossip/src/crds_value.rs

@@ -9,6 +9,7 @@ use {
     bincode::{serialize, serialized_size},
     rand::{CryptoRng, Rng},
     serde::de::{Deserialize, Deserializer},
+    solana_runtime::vote_parser,
     solana_sdk::{
         clock::Slot,
         hash::Hash,
@@ -18,7 +19,6 @@ use {
         timing::timestamp,
         transaction::Transaction,
     },
-    solana_vote_program::vote_transaction::parse_vote_transaction,
     std::{
         borrow::{Borrow, Cow},
         cmp::Ordering,
@@ -307,7 +307,7 @@ impl Sanitize for Vote {
 impl Vote {
     // Returns None if cannot parse transaction into a vote.
     pub fn new(from: Pubkey, transaction: Transaction, wallclock: u64) -> Option<Self> {
-        parse_vote_transaction(&transaction).map(|(_, vote, _)| Self {
+        vote_parser::parse_vote_transaction(&transaction).map(|(_, vote, _)| Self {
             from,
             transaction,
             wallclock,

local-cluster/tests/local_cluster_slow.rs

@@ -28,6 +28,7 @@ use {
         local_cluster::{ClusterConfig, LocalCluster},
         validator_configs::*,
     },
+    solana_runtime::vote_parser,
     solana_sdk::{
         clock::{Slot, MAX_PROCESSING_AGE},
         hash::Hash,
@@ -499,7 +500,7 @@ fn test_duplicate_shreds_broadcast_leader() {
                     .filter_map(|(label, leader_vote_tx)| {
                         // Filter out votes not from the bad leader
                         if label.pubkey() == bad_leader_id {
-                            let vote = vote_transaction::parse_vote_transaction(&leader_vote_tx)
+                            let vote = vote_parser::parse_vote_transaction(&leader_vote_tx)
                                 .map(|(_, vote, _)| vote)
                                 .unwrap();
                             // Filter out empty votes

programs/vote/src/vote_transaction.rs

@@ -1,76 +1,13 @@
 use {
-    crate::{
+    crate::{vote_instruction, vote_state::Vote},
-        vote_instruction::{self, VoteInstruction},
-        vote_state::Vote,
-    },
     solana_sdk::{
         clock::Slot,
         hash::Hash,
-        instruction::CompiledInstruction,
-        program_utils::limited_deserialize,
-        pubkey::Pubkey,
         signature::{Keypair, Signer},
-        transaction::{SanitizedTransaction, Transaction},
+        transaction::Transaction,
     },
 };
 
-pub type ParsedVote = (Pubkey, Vote, Option<Hash>);
-
-fn parse_vote(vote_ix: &CompiledInstruction, vote_key: &Pubkey) -> Option<ParsedVote> {
-    let vote_instruction = limited_deserialize(&vote_ix.data).ok();
-    vote_instruction.and_then(|vote_instruction| match vote_instruction {
-        VoteInstruction::Vote(vote) => Some((*vote_key, vote, None)),
-        VoteInstruction::VoteSwitch(vote, hash) => Some((*vote_key, vote, Some(hash))),
-        _ => None,
-    })
-}
-
-pub fn parse_sanitized_vote_transaction(tx: &SanitizedTransaction) -> Option<ParsedVote> {
-    // Check first instruction for a vote
-    let message = tx.message();
-    message
-        .program_instructions_iter()
-        .next()
-        .and_then(|(program_id, first_ix)| {
-            if !crate::check_id(program_id) {
-                return None;
-            }
-
-            first_ix.accounts.first().and_then(|first_account| {
-                message
-                    .get_account_key(*first_account as usize)
-                    .and_then(|key| parse_vote(first_ix, key))
-            })
-        })
-}
-
-pub fn parse_vote_transaction(tx: &Transaction) -> Option<ParsedVote> {
-    // Check first instruction for a vote
-    let message = tx.message();
-    message.instructions.get(0).and_then(|first_instruction| {
-        let prog_id_idx = first_instruction.program_id_index as usize;
-        match message.account_keys.get(prog_id_idx) {
-            Some(program_id) => {
-                if !crate::check_id(program_id) {
-                    return None;
-                }
-            }
-            _ => {
-                return None;
-            }
-        };
-        first_instruction
-            .accounts
-            .first()
-            .and_then(|first_account| {
-                message
-                    .account_keys
-                    .get(*first_account as usize)
-                    .and_then(|key| parse_vote(first_instruction, key))
-            })
-    })
-}
-
 pub fn new_vote_transaction(
     slots: Vec<Slot>,
     bank_hash: Hash,
@@ -102,44 +39,3 @@ pub fn new_vote_transaction(
     vote_tx.partial_sign(&[authorized_voter_keypair], blockhash);
     vote_tx
 }
-
-#[cfg(test)]
-mod test {
-    use {super::*, solana_sdk::hash::hash};
-
-    fn run_test_parse_vote_transaction(input_hash: Option<Hash>) {
-        let node_keypair = Keypair::new();
-        let vote_keypair = Keypair::new();
-        let auth_voter_keypair = Keypair::new();
-        let bank_hash = Hash::default();
-        let vote_tx = new_vote_transaction(
-            vec![42],
-            bank_hash,
-            Hash::default(),
-            &node_keypair,
-            &vote_keypair,
-            &auth_voter_keypair,
-            input_hash,
-        );
-        let (key, vote, hash) = parse_vote_transaction(&vote_tx).unwrap();
-        assert_eq!(hash, input_hash);
-        assert_eq!(vote, Vote::new(vec![42], bank_hash));
-        assert_eq!(key, vote_keypair.pubkey());
-
-        // Test bad program id fails
-        let mut vote_ix = vote_instruction::vote(
-            &vote_keypair.pubkey(),
-            &auth_voter_keypair.pubkey(),
-            Vote::new(vec![1, 2], Hash::default()),
-        );
-        vote_ix.program_id = Pubkey::default();
-        let vote_tx = Transaction::new_with_payer(&[vote_ix], Some(&node_keypair.pubkey()));
-        assert!(parse_vote_transaction(&vote_tx).is_none());
-    }
-
-    #[test]
-    fn test_parse_vote_transaction() {
-        run_test_parse_vote_transaction(None);
-        run_test_parse_vote_transaction(Some(hash(&[42u8])));
-    }
-}

runtime/src/bank.rs

@@ -64,6 +64,7 @@ use {
         system_instruction_processor::{get_system_account_kind, SystemAccountKind},
         transaction_batch::TransactionBatch,
         vote_account::VoteAccount,
+        vote_parser,
     },
     byteorder::{ByteOrder, LittleEndian},
     dashmap::DashMap,
@@ -122,7 +123,6 @@ use {
         nonce, nonce_account,
         packet::PACKET_DATA_SIZE,
         precompiles::get_precompiles,
-        program_utils::limited_deserialize,
         pubkey::Pubkey,
         saturating_add_assign, secp256k1_program,
         signature::{Keypair, Signature},
@@ -139,10 +139,7 @@ use {
     solana_stake_program::stake_state::{
         self, InflationPointCalculationEvent, PointValue, StakeState,
     },
-    solana_vote_program::{
+    solana_vote_program::vote_state::{VoteState, VoteStateVersions},
-        vote_instruction::VoteInstruction,
-        vote_state::{VoteState, VoteStateVersions},
-    },
     std::{
         borrow::Cow,
         cell::RefCell,
@@ -3982,7 +3979,7 @@ impl Bank {
                     }
                 }
 
-                let is_vote = is_simple_vote_transaction(tx);
+                let is_vote = vote_parser::is_simple_vote_transaction(tx);
                 let store = match transaction_log_collector_config.filter {
                     TransactionLogCollectorFilter::All => {
                         !is_vote || !filtered_mentioned_addresses.is_empty()
@@ -6438,26 +6435,6 @@ pub fn goto_end_of_slot(bank: &mut Bank) {
     }
 }
 
-fn is_simple_vote_transaction(transaction: &SanitizedTransaction) -> bool {
-    if transaction.message().instructions().len() == 1 {
-        let (program_pubkey, instruction) = transaction
-            .message()
-            .program_instructions_iter()
-            .next()
-            .unwrap();
-        if program_pubkey == &solana_vote_program::id() {
-            if let Ok(vote_instruction) = limited_deserialize::<VoteInstruction>(&instruction.data)
-            {
-                return matches!(
-                    vote_instruction,
-                    VoteInstruction::Vote(_) | VoteInstruction::VoteSwitch(_, _)
-                );
-            }
-        }
-    }
-    false
-}
-
 #[cfg(test)]
 pub(crate) mod tests {
     #[allow(deprecated)]

runtime/src/bank_utils.rs

@@ -2,10 +2,10 @@ use {
     crate::{
         bank::{Bank, TransactionResults},
         genesis_utils::{self, GenesisConfigInfo, ValidatorVoteKeypairs},
+        vote_parser,
         vote_sender_types::ReplayVoteSender,
     },
     solana_sdk::{pubkey::Pubkey, signature::Signer, transaction::SanitizedTransaction},
-    solana_vote_program::vote_transaction,
 };
 
 pub fn setup_bank_and_vote_pubkeys_for_tests(
@@ -45,9 +45,7 @@ pub fn find_and_send_votes(
             .zip(execution_results.iter())
             .for_each(|(tx, result)| {
                 if tx.is_simple_vote_transaction() && result.was_executed_successfully() {
-                    if let Some(parsed_vote) =
+                    if let Some(parsed_vote) = vote_parser::parse_sanitized_vote_transaction(tx) {
-                        vote_transaction::parse_sanitized_vote_transaction(tx)
-                    {
                         if parsed_vote.1.slots.last().is_some() {
                             let _ = vote_sender.send(parsed_vote);
                         }

runtime/src/lib.rs

@@ -56,6 +56,7 @@ pub mod status_cache;
 mod system_instruction_processor;
 pub mod transaction_batch;
 pub mod vote_account;
+pub mod vote_parser;
 pub mod vote_sender_types;
 pub mod waitable_condvar;
 

runtime/src/vote_parser.rs

@@ -0,0 +1,116 @@
+use {
+    solana_sdk::{
+        hash::Hash,
+        program_utils::limited_deserialize,
+        pubkey::Pubkey,
+        transaction::{SanitizedTransaction, Transaction},
+    },
+    solana_vote_program::{vote_instruction::VoteInstruction, vote_state::Vote},
+};
+
+pub type ParsedVote = (Pubkey, Vote, Option<Hash>);
+
+// Used for filtering out votes from the transaction log collector
+pub(crate) fn is_simple_vote_transaction(transaction: &SanitizedTransaction) -> bool {
+    if transaction.message().instructions().len() == 1 {
+        let (program_pubkey, instruction) = transaction
+            .message()
+            .program_instructions_iter()
+            .next()
+            .unwrap();
+        if program_pubkey == &solana_vote_program::id() {
+            if let Ok(vote_instruction) = limited_deserialize::<VoteInstruction>(&instruction.data)
+            {
+                return matches!(
+                    vote_instruction,
+                    VoteInstruction::Vote(_) | VoteInstruction::VoteSwitch(_, _)
+                );
+            }
+        }
+    }
+    false
+}
+
+// Used for locally forwarding processed vote transactions to consensus
+pub fn parse_sanitized_vote_transaction(tx: &SanitizedTransaction) -> Option<ParsedVote> {
+    // Check first instruction for a vote
+    let message = tx.message();
+    let (program_id, first_instruction) = message.program_instructions_iter().next()?;
+    if !solana_vote_program::check_id(program_id) {
+        return None;
+    }
+    let first_account = usize::from(*first_instruction.accounts.first()?);
+    let key = message.get_account_key(first_account)?;
+    let (vote, switch_proof_hash) = parse_vote_instruction_data(&first_instruction.data)?;
+    Some((*key, vote, switch_proof_hash))
+}
+
+// Used for parsing gossip vote transactions
+pub fn parse_vote_transaction(tx: &Transaction) -> Option<ParsedVote> {
+    // Check first instruction for a vote
+    let message = tx.message();
+    let first_instruction = message.instructions.first()?;
+    let program_id_index = usize::from(first_instruction.program_id_index);
+    let program_id = message.account_keys.get(program_id_index)?;
+    if !solana_vote_program::check_id(program_id) {
+        return None;
+    }
+    let first_account = usize::from(*first_instruction.accounts.first()?);
+    let key = message.account_keys.get(first_account)?;
+    let (vote, switch_proof_hash) = parse_vote_instruction_data(&first_instruction.data)?;
+    Some((*key, vote, switch_proof_hash))
+}
+
+fn parse_vote_instruction_data(vote_instruction_data: &[u8]) -> Option<(Vote, Option<Hash>)> {
+    match limited_deserialize(vote_instruction_data).ok()? {
+        VoteInstruction::Vote(vote) => Some((vote, None)),
+        VoteInstruction::VoteSwitch(vote, hash) => Some((vote, Some(hash))),
+        _ => None,
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use solana_sdk::signature::{Keypair, Signer};
+    use solana_vote_program::{
+        vote_instruction, vote_state::Vote, vote_transaction::new_vote_transaction,
+    };
+
+    use {super::*, solana_sdk::hash::hash};
+
+    fn run_test_parse_vote_transaction(input_hash: Option<Hash>) {
+        let node_keypair = Keypair::new();
+        let vote_keypair = Keypair::new();
+        let auth_voter_keypair = Keypair::new();
+        let bank_hash = Hash::default();
+        let vote_tx = new_vote_transaction(
+            vec![42],
+            bank_hash,
+            Hash::default(),
+            &node_keypair,
+            &vote_keypair,
+            &auth_voter_keypair,
+            input_hash,
+        );
+        let (key, vote, hash) = parse_vote_transaction(&vote_tx).unwrap();
+        assert_eq!(hash, input_hash);
+        assert_eq!(vote, Vote::new(vec![42], bank_hash));
+        assert_eq!(key, vote_keypair.pubkey());
+
+        // Test bad program id fails
+        let mut vote_ix = vote_instruction::vote(
+            &vote_keypair.pubkey(),
+            &auth_voter_keypair.pubkey(),
+            Vote::new(vec![1, 2], Hash::default()),
+        );
+        vote_ix.program_id = Pubkey::default();
+        let vote_tx = Transaction::new_with_payer(&[vote_ix], Some(&node_keypair.pubkey()));
+        assert!(parse_vote_transaction(&vote_tx).is_none());
+    }
+
+    #[test]
+    fn test_parse_vote_transaction() {
+        run_test_parse_vote_transaction(None);
+        run_test_parse_vote_transaction(Some(hash(&[42u8])));
+    }
+}

sdk/src/transaction/sanitized.rs

@@ -61,6 +61,7 @@ impl SanitizedTransaction {
         };
 
         let is_simple_vote_tx = is_simple_vote_tx.unwrap_or_else(|| {
+            // TODO: Move to `vote_parser` runtime module
             let mut ix_iter = message.program_instructions_iter();
             ix_iter.next().map(|(program_id, _ix)| program_id) == Some(&crate::vote::program::id())
         });