discards serialized gossip crds votes if cannot parse tx (#22129)

gossip/src/cluster_info.rs

@@ -983,7 +983,7 @@ impl ClusterInfo {
         assert!((vote_index as usize) < MAX_LOCKOUT_HISTORY);
         let self_pubkey = self.id();
         let now = timestamp();
-        let vote = Vote::new(self_pubkey, vote, now);
+        let vote = Vote::new(self_pubkey, vote, now).unwrap();
         let vote = CrdsData::Vote(vote_index, vote);
         let vote = CrdsValue::new_signed(vote, &self.keypair());
         let mut gossip_crds = self.gossip.crds.write().unwrap();
@@ -4219,7 +4219,8 @@ mod tests {
             keypair.pubkey(),
             vote_tx,
             0, // wallclock
-        );
+        )
+        .unwrap();
         let vote = CrdsValue::new_signed(CrdsData::Vote(1, vote), &Keypair::new());
         assert!(bincode::serialized_size(&vote).unwrap() <= PUSH_MESSAGE_MAX_PAYLOAD_SIZE as u64);
     }

gossip/src/crds_gossip_pull.rs

@@ -675,7 +675,7 @@ pub(crate) mod tests {
         rand::{seq::SliceRandom, thread_rng, SeedableRng},
         rand_chacha::ChaChaRng,
         rayon::ThreadPoolBuilder,
-        solana_perf::test_tx::test_tx,
+        solana_perf::test_tx::new_test_vote_tx,
         solana_sdk::{
             hash::{hash, HASH_BYTES},
             packet::PACKET_DATA_SIZE,
@@ -1623,6 +1623,7 @@ pub(crate) mod tests {
 
     #[test]
     fn test_process_pull_response() {
+        let mut rng = rand::thread_rng();
         let node_crds = RwLock::<Crds>::default();
         let node = CrdsGossipPull::default();
 
@@ -1678,8 +1679,8 @@ pub(crate) mod tests {
         );
 
         // construct something that's not a contact info
-        let peer_vote =
+        let peer_vote = Vote::new(peer_pubkey, new_test_vote_tx(&mut rng), 0).unwrap();
-            CrdsValue::new_unsigned(CrdsData::Vote(0, Vote::new(peer_pubkey, test_tx(), 0)));
+        let peer_vote = CrdsValue::new_unsigned(CrdsData::Vote(0, peer_vote));
         // check that older CrdsValues (non-ContactInfos) infos pass even if are too old,
         // but a recent contact info (inserted above) exists
         assert_eq!(

gossip/src/crds_value.rs

@@ -305,15 +305,14 @@ impl Sanitize for Vote {
 }
 
 impl Vote {
-    pub fn new(from: Pubkey, transaction: Transaction, wallclock: u64) -> Self {
+    // Returns None if cannot parse transaction into a vote.
-        let slot =
+    pub fn new(from: Pubkey, transaction: Transaction, wallclock: u64) -> Option<Self> {
-            parse_vote_transaction(&transaction).and_then(|(_, vote, _)| vote.last_voted_slot());
+        parse_vote_transaction(&transaction).map(|(_, vote, _)| Self {
-        Self {
             from,
             transaction,
             wallclock,
-            slot,
+            slot: vote.last_voted_slot(),
-        }
+        })
     }
 
     /// New random Vote for tests and benchmarks.
@@ -347,16 +346,11 @@ impl<'de> Deserialize<'de> for Vote {
             wallclock: u64,
         }
         let vote = Vote::deserialize(deserializer)?;
-        let vote = match vote.transaction.sanitize() {
+        vote.transaction
-            Ok(_) => Self::new(vote.from, vote.transaction, vote.wallclock),
+            .sanitize()
-            Err(_) => Self {
+            .map_err(serde::de::Error::custom)?;
-                from: vote.from,
+        Self::new(vote.from, vote.transaction, vote.wallclock)
-                transaction: vote.transaction,
+            .ok_or_else(|| serde::de::Error::custom("invalid vote tx"))
-                wallclock: vote.wallclock,
-                slot: None,
-            },
-        };
-        Ok(vote)
     }
 }
 
@@ -692,7 +686,7 @@ mod test {
         bincode::{deserialize, Options},
         rand::SeedableRng,
         rand_chacha::ChaChaRng,
-        solana_perf::test_tx::test_tx,
+        solana_perf::test_tx::new_test_vote_tx,
         solana_sdk::{
             signature::{Keypair, Signer},
             timing::timestamp,
@@ -703,15 +697,14 @@ mod test {
 
     #[test]
     fn test_keys_and_values() {
+        let mut rng = rand::thread_rng();
         let v = CrdsValue::new_unsigned(CrdsData::ContactInfo(ContactInfo::default()));
         assert_eq!(v.wallclock(), 0);
         let key = v.contact_info().unwrap().id;
         assert_eq!(v.label(), CrdsValueLabel::ContactInfo(key));
 
-        let v = CrdsValue::new_unsigned(CrdsData::Vote(
+        let v = Vote::new(Pubkey::default(), new_test_vote_tx(&mut rng), 0).unwrap();
-            0,
+        let v = CrdsValue::new_unsigned(CrdsData::Vote(0, v));
-            Vote::new(Pubkey::default(), test_tx(), 0),
-        ));
         assert_eq!(v.wallclock(), 0);
         let key = match &v.data {
             CrdsData::Vote(_, vote) => vote.from,
@@ -759,6 +752,7 @@ mod test {
 
     #[test]
     fn test_signature() {
+        let mut rng = rand::thread_rng();
         let keypair = Keypair::new();
         let wrong_keypair = Keypair::new();
         let mut v = CrdsValue::new_unsigned(CrdsData::ContactInfo(ContactInfo::new_localhost(
@@ -766,10 +760,8 @@ mod test {
             timestamp(),
         )));
         verify_signatures(&mut v, &keypair, &wrong_keypair);
-        v = CrdsValue::new_unsigned(CrdsData::Vote(
+        let v = Vote::new(keypair.pubkey(), new_test_vote_tx(&mut rng), timestamp()).unwrap();
-            0,
+        let mut v = CrdsValue::new_unsigned(CrdsData::Vote(0, v));
-            Vote::new(keypair.pubkey(), test_tx(), timestamp()),
-        ));
         verify_signatures(&mut v, &keypair, &wrong_keypair);
         v = CrdsValue::new_unsigned(CrdsData::LowestSlot(
             0,
@@ -780,14 +772,10 @@ mod test {
 
     #[test]
     fn test_max_vote_index() {
+        let mut rng = rand::thread_rng();
         let keypair = Keypair::new();
-        let vote = CrdsValue::new_signed(
+        let vote = Vote::new(keypair.pubkey(), new_test_vote_tx(&mut rng), timestamp()).unwrap();
-            CrdsData::Vote(
+        let vote = CrdsValue::new_signed(CrdsData::Vote(MAX_VOTES, vote), &keypair);
-                MAX_VOTES,
-                Vote::new(keypair.pubkey(), test_tx(), timestamp()),
-            ),
-            &keypair,
-        );
         assert!(vote.sanitize().is_err());
     }
 
@@ -811,7 +799,8 @@ mod test {
             Pubkey::new_unique(), // from
             tx,
             rng.gen(), // wallclock
-        );
+        )
+        .unwrap();
         assert_eq!(vote.slot, Some(7));
         let bytes = bincode::serialize(&vote).unwrap();
         let other = bincode::deserialize(&bytes[..]).unwrap();

local-cluster/src/cluster_tests.rs

@@ -441,7 +441,7 @@ pub fn submit_vote_to_cluster_gossip(
         vec![CrdsValue::new_signed(
             CrdsData::Vote(
                 0,
-                crds_value::Vote::new(node_keypair.pubkey(), vote_tx, timestamp()),
+                crds_value::Vote::new(node_keypair.pubkey(), vote_tx, timestamp()).unwrap(),
             ),
             node_keypair,
         )],

perf/src/sigverify.rs

@@ -599,7 +599,7 @@ mod tests {
         crate::{
             packet::{Packet, PacketBatch},
             sigverify::{self, PacketOffsets},
-            test_tx::{test_multisig_tx, test_tx, vote_tx},
+            test_tx::{new_test_vote_tx, test_multisig_tx, test_tx},
         },
         bincode::{deserialize, serialize},
         solana_sdk::{
@@ -1187,6 +1187,7 @@ mod tests {
     #[test]
     fn test_is_simple_vote_transaction() {
         solana_logger::setup();
+        let mut rng = rand::thread_rng();
 
         // tansfer tx is not
         {
@@ -1200,7 +1201,7 @@ mod tests {
 
         // single vote tx is
         {
-            let mut tx = vote_tx();
+            let mut tx = new_test_vote_tx(&mut rng);
             tx.message.instructions[0].data = vec![1, 2, 3];
             let mut packet = sigverify::make_packet_from_transaction(tx);
             let packet_offsets = do_get_packet_offsets(&packet, 0).unwrap();
@@ -1233,15 +1234,17 @@ mod tests {
     #[test]
     fn test_is_simple_vote_transaction_with_offsets() {
         solana_logger::setup();
+        let mut rng = rand::thread_rng();
 
         let mut current_offset = 0usize;
         let mut batch = PacketBatch::default();
         batch
             .packets
             .push(sigverify::make_packet_from_transaction(test_tx()));
+        let tx = new_test_vote_tx(&mut rng);
         batch
             .packets
-            .push(sigverify::make_packet_from_transaction(vote_tx()));
+            .push(sigverify::make_packet_from_transaction(tx));
         batch
             .packets
             .iter_mut()

perf/src/test_tx.rs

@@ -1,5 +1,7 @@
 use {
+    rand::{CryptoRng, Rng, RngCore},
     solana_sdk::{
+        clock::Slot,
         hash::Hash,
         instruction::CompiledInstruction,
         signature::{Keypair, Signer},
@@ -50,15 +52,21 @@ pub fn test_multisig_tx() -> Transaction {
     )
 }
 
-pub fn vote_tx() -> Transaction {
+pub fn new_test_vote_tx<R>(rng: &mut R) -> Transaction
-    let keypair = Keypair::new();
+where
+    R: CryptoRng + RngCore,
+{
+    let mut slots: Vec<Slot> = std::iter::repeat_with(|| rng.gen()).take(5).collect();
+    slots.sort_unstable();
+    slots.dedup();
+    let switch_proof_hash = rng.gen_bool(0.5).then(|| solana_sdk::hash::new_rand(rng));
     vote_transaction::new_vote_transaction(
-        vec![2],
+        slots,
-        Hash::default(),
+        solana_sdk::hash::new_rand(rng), // bank_hash
-        Hash::default(),
+        solana_sdk::hash::new_rand(rng), // blockhash
-        &keypair,
+        &Keypair::generate(rng),         // node_keypair
-        &keypair,
+        &Keypair::generate(rng),         // vote_keypair
-        &keypair,
+        &Keypair::generate(rng),         // authorized_voter_keypair
-        None,
+        switch_proof_hash,
     )
 }