uses varint encoding for vote-state lockout offsets

The commit removes CompactVoteStateUpdate and instead reduces serialized size of VoteStateUpdate using varint encoding for vote-state lockout offsets.
yu3ver · Sep 12, 2022 · 4f22ee8 · 4f22ee8
1 parent f6fbc47
commit 4f22ee8
Show file tree

Hide file tree

Showing 8 changed files with 192 additions and 396 deletions.
diff --git a/core/src/consensus.rs b/core/src/consensus.rs
@@ -169,7 +169,7 @@ impl TowerVersions {
     }
 }
 
-#[frozen_abi(digest = "GrkFcKqGEkJNUYoK1M8rorehi2yyLF4N3Gsj6j8f47Jn")]
+#[frozen_abi(digest = "HQoLKAJEQTuVy8nMSkVWbrH3M5xKksxdMEZHGLWbnX6w")]
 #[derive(Clone, Serialize, Deserialize, Debug, PartialEq, AbiExample)]
 pub struct Tower {
     pub node_pubkey: Pubkey,

diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs
@@ -2020,13 +2020,7 @@ impl ReplayStage {
             .is_active(&feature_set::compact_vote_state_updates::id());
         let vote = match (should_compact, vote) {
             (true, VoteTransaction::VoteStateUpdate(vote_state_update)) => {
-                if let Some(compact_vote_state_update) = vote_state_update.compact() {
-                    VoteTransaction::from(compact_vote_state_update)
-                } else {
-                    // Compaction failed
-                    warn!("Compaction failed when generating vote tx for vote account {}. Unable to vote", vote_account_pubkey);
-                    return None;
-                }
+                VoteTransaction::CompactVoteStateUpdate(vote_state_update)
             }
             (_, vote) => vote,
         };

diff --git a/programs/vote/src/vote_processor.rs b/programs/vote/src/vote_processor.rs
@@ -172,8 +172,8 @@ pub fn process_instruction(
                 Err(InstructionError::InvalidInstructionData)
             }
         }
-        VoteInstruction::CompactUpdateVoteState(compact_vote_state_update)
-        | VoteInstruction::CompactUpdateVoteStateSwitch(compact_vote_state_update, _) => {
+        VoteInstruction::CompactUpdateVoteState(vote_state_update)
+        | VoteInstruction::CompactUpdateVoteStateSwitch(vote_state_update, _) => {
             if invoke_context
                 .feature_set
                 .is_active(&feature_set::allow_votes_to_directly_update_vote_state::id())
@@ -184,7 +184,6 @@ pub fn process_instruction(
                 let sysvar_cache = invoke_context.get_sysvar_cache();
                 let slot_hashes = sysvar_cache.get_slot_hashes()?;
                 let clock = sysvar_cache.get_clock()?;
-                let vote_state_update = compact_vote_state_update.uncompact()?;
                 vote_state::process_vote_state_update(
                     &mut me,
                     slot_hashes.slot_hashes(),

diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs
@@ -5,7 +5,7 @@ use {
     log::*,
     serde_derive::{Deserialize, Serialize},
     solana_metrics::datapoint_debug,
-    solana_program::vote::{error::VoteError, program::id},
+    solana_program::vote::{error::VoteError, program::id, state::serde_compact_vote_state_update},
     solana_sdk::{
         account::{AccountSharedData, ReadableAccount, WritableAccount},
         clock::{Epoch, Slot, UnixTimestamp},
@@ -27,98 +27,89 @@ use {
     },
 };
 
-#[frozen_abi(digest = "8Xa47j7LCp99Q7CQeTz4KPWU8sZgGFpAJw2K4VbPgGh8")]
+#[frozen_abi(digest = "2AuJFjx7SYrJ2ugCfH1jFh3Lr9UHMEPfKwwk1NcjqND1")]
 #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize, AbiEnumVisitor, AbiExample)]
 pub enum VoteTransaction {
     Vote(Vote),
     VoteStateUpdate(VoteStateUpdate),
-    CompactVoteStateUpdate(CompactVoteStateUpdate),
+    #[serde(with = "serde_compact_vote_state_update")]
+    CompactVoteStateUpdate(VoteStateUpdate),
 }
 
 impl VoteTransaction {
     pub fn slots(&self) -> Vec<Slot> {
         match self {
             VoteTransaction::Vote(vote) => vote.slots.clone(),
             VoteTransaction::VoteStateUpdate(vote_state_update) => vote_state_update.slots(),
-            VoteTransaction::CompactVoteStateUpdate(compact_state_update) => {
-                compact_state_update.slots()
-            }
+            VoteTransaction::CompactVoteStateUpdate(vote_state_update) => vote_state_update.slots(),
         }
     }
 
     pub fn slot(&self, i: usize) -> Slot {
         match self {
             VoteTransaction::Vote(vote) => vote.slots[i],
-            VoteTransaction::VoteStateUpdate(vote_state_update) => {
+            VoteTransaction::VoteStateUpdate(vote_state_update)
+            | VoteTransaction::CompactVoteStateUpdate(vote_state_update) => {
                 vote_state_update.lockouts[i].slot
             }
-            VoteTransaction::CompactVoteStateUpdate(compact_state_update) => {
-                compact_state_update.slots()[i]
-            }
         }
     }
 
     pub fn len(&self) -> usize {
         match self {
             VoteTransaction::Vote(vote) => vote.slots.len(),
-            VoteTransaction::VoteStateUpdate(vote_state_update) => vote_state_update.lockouts.len(),
-            VoteTransaction::CompactVoteStateUpdate(compact_state_update) => {
-                1 + compact_state_update.lockouts_32.len()
-                    + compact_state_update.lockouts_16.len()
-                    + compact_state_update.lockouts_8.len()
+            VoteTransaction::VoteStateUpdate(vote_state_update)
+            | VoteTransaction::CompactVoteStateUpdate(vote_state_update) => {
+                vote_state_update.lockouts.len()
             }
         }
     }
 
     pub fn is_empty(&self) -> bool {
         match self {
             VoteTransaction::Vote(vote) => vote.slots.is_empty(),
-            VoteTransaction::VoteStateUpdate(vote_state_update) => {
+            VoteTransaction::VoteStateUpdate(vote_state_update)
+            | VoteTransaction::CompactVoteStateUpdate(vote_state_update) => {
                 vote_state_update.lockouts.is_empty()
             }
-            VoteTransaction::CompactVoteStateUpdate(_) => false,
         }
     }
 
     pub fn hash(&self) -> Hash {
         match self {
             VoteTransaction::Vote(vote) => vote.hash,
             VoteTransaction::VoteStateUpdate(vote_state_update) => vote_state_update.hash,
-            VoteTransaction::CompactVoteStateUpdate(compact_state_update) => {
-                compact_state_update.hash
-            }
+            VoteTransaction::CompactVoteStateUpdate(vote_state_update) => vote_state_update.hash,
         }
     }
 
     pub fn timestamp(&self) -> Option<UnixTimestamp> {
         match self {
             VoteTransaction::Vote(vote) => vote.timestamp,
-            VoteTransaction::VoteStateUpdate(vote_state_update) => vote_state_update.timestamp,
-            VoteTransaction::CompactVoteStateUpdate(compact_state_update) => {
-                compact_state_update.timestamp
+            VoteTransaction::VoteStateUpdate(vote_state_update)
+            | VoteTransaction::CompactVoteStateUpdate(vote_state_update) => {
+                vote_state_update.timestamp
             }
         }
     }
 
     pub fn set_timestamp(&mut self, ts: Option<UnixTimestamp>) {
         match self {
             VoteTransaction::Vote(vote) => vote.timestamp = ts,
-            VoteTransaction::VoteStateUpdate(vote_state_update) => vote_state_update.timestamp = ts,
-            VoteTransaction::CompactVoteStateUpdate(compact_state_update) => {
-                compact_state_update.timestamp = ts
+            VoteTransaction::VoteStateUpdate(vote_state_update)
+            | VoteTransaction::CompactVoteStateUpdate(vote_state_update) => {
+                vote_state_update.timestamp = ts
             }
         }
     }
 
     pub fn last_voted_slot(&self) -> Option<Slot> {
         match self {
             VoteTransaction::Vote(vote) => vote.slots.last().copied(),
-            VoteTransaction::VoteStateUpdate(vote_state_update) => {
+            VoteTransaction::VoteStateUpdate(vote_state_update)
+            | VoteTransaction::CompactVoteStateUpdate(vote_state_update) => {
                 Some(vote_state_update.lockouts.back()?.slot)
             }
-            VoteTransaction::CompactVoteStateUpdate(compact_state_update) => {
-                compact_state_update.slots().last().copied()
-            }
         }
     }
 
@@ -139,12 +130,6 @@ impl From<VoteStateUpdate> for VoteTransaction {
     }
 }
 
-impl From<CompactVoteStateUpdate> for VoteTransaction {
-    fn from(compact_state_update: CompactVoteStateUpdate) -> Self {
-        VoteTransaction::CompactVoteStateUpdate(compact_state_update)
-    }
-}
-
 // utility function, used by Stakes, tests
 pub fn from<T: ReadableAccount>(account: &T) -> Option<VoteState> {
     VoteState::deserialize(account.data()).ok()
@@ -2970,103 +2955,4 @@ mod tests {
             Err(VoteError::SlotHashMismatch),
         );
     }
-
-    #[test]
-    fn test_compact_vote_state_update_parity() {
-        let mut vote_state_update = VoteStateUpdate::from(vec![(2, 4), (4, 3), (6, 2), (7, 1)]);
-        vote_state_update.hash = Hash::new_unique();
-        vote_state_update.root = Some(1);
-
-        let compact_vote_state_update = vote_state_update.clone().compact().unwrap();
-
-        assert_eq!(vote_state_update.slots(), compact_vote_state_update.slots());
-        assert_eq!(vote_state_update.hash, compact_vote_state_update.hash);
-        assert_eq!(vote_state_update.root, compact_vote_state_update.root());
-
-        let vote_state_update_new = compact_vote_state_update.uncompact().unwrap();
-        assert_eq!(vote_state_update, vote_state_update_new);
-    }
-
-    #[test]
-    fn test_compact_vote_state_update_large_offsets() {
-        let vote_state_update = VoteStateUpdate::from(vec![
-            (0, 31),
-            (1, 30),
-            (2, 29),
-            (3, 28),
-            (u64::pow(2, 28), 17),
-            (u64::pow(2, 28) + u64::pow(2, 16), 1),
-        ]);
-        let compact_vote_state_update = vote_state_update.clone().compact().unwrap();
-
-        assert_eq!(vote_state_update.slots(), compact_vote_state_update.slots());
-
-        let vote_state_update_new = compact_vote_state_update.uncompact().unwrap();
-        assert_eq!(vote_state_update, vote_state_update_new);
-    }
-
-    #[test]
-    fn test_compact_vote_state_update_border_conditions() {
-        let two_31 = u64::pow(2, 31);
-        let two_15 = u64::pow(2, 15);
-        let vote_state_update = VoteStateUpdate::from(vec![
-            (0, 31),
-            (two_31, 16),
-            (two_31 + 1, 15),
-            (two_31 + two_15, 7),
-            (two_31 + two_15 + 1, 6),
-            (two_31 + two_15 + 1 + 64, 1),
-        ]);
-        let compact_vote_state_update = vote_state_update.clone().compact().unwrap();
-
-        assert_eq!(vote_state_update.slots(), compact_vote_state_update.slots());
-
-        let vote_state_update_new = compact_vote_state_update.uncompact().unwrap();
-        assert_eq!(vote_state_update, vote_state_update_new);
-    }
-
-    #[test]
-    fn test_compact_vote_state_update_large_root() {
-        let two_58 = u64::pow(2, 58);
-        let two_31 = u64::pow(2, 31);
-        let mut vote_state_update = VoteStateUpdate::from(vec![(two_58, 31), (two_58 + two_31, 1)]);
-        vote_state_update.root = Some(two_31);
-        let compact_vote_state_update = vote_state_update.clone().compact().unwrap();
-
-        assert_eq!(vote_state_update.slots(), compact_vote_state_update.slots());
-
-        let vote_state_update_new = compact_vote_state_update.uncompact().unwrap();
-        assert_eq!(vote_state_update, vote_state_update_new);
-    }
-
-    #[test]
-    fn test_compact_vote_state_update_overflow() {
-        let compact_vote_state_update = CompactVoteStateUpdate {
-            root: u64::MAX - 1,
-            root_to_first_vote_offset: 10,
-            lockouts_32: vec![],
-            lockouts_16: vec![],
-            lockouts_8: vec![CompactLockout::new(10)],
-            hash: Hash::new_unique(),
-            timestamp: None,
-        };
-        assert_eq!(
-            Err(InstructionError::ArithmeticOverflow),
-            compact_vote_state_update.uncompact()
-        );
-
-        let compact_vote_state_update = CompactVoteStateUpdate {
-            root: u64::MAX - u32::MAX as u64,
-            root_to_first_vote_offset: 10,
-            lockouts_32: vec![CompactLockout::new(u32::MAX)],
-            lockouts_16: vec![],
-            lockouts_8: vec![CompactLockout::new(10)],
-            hash: Hash::new_unique(),
-            timestamp: None,
-        };
-        assert_eq!(
-            Err(InstructionError::ArithmeticOverflow),
-            compact_vote_state_update.uncompact()
-        );
-    }
 }
diff --git a/runtime/src/vote_parser.rs b/runtime/src/vote_parser.rs
@@ -82,17 +82,11 @@ fn parse_vote_instruction_data(
         VoteInstruction::UpdateVoteStateSwitch(vote_state_update, hash) => {
             Some((VoteTransaction::from(vote_state_update), Some(hash)))
         }
-        VoteInstruction::CompactUpdateVoteState(compact_vote_state_update) => {
-            compact_vote_state_update
-                .uncompact()
-                .ok()
-                .map(|vote_state_update| (VoteTransaction::from(vote_state_update), None))
+        VoteInstruction::CompactUpdateVoteState(vote_state_update) => {
+            Some((VoteTransaction::from(vote_state_update), None))
         }
-        VoteInstruction::CompactUpdateVoteStateSwitch(compact_vote_state_update, hash) => {
-            compact_vote_state_update
-                .uncompact()
-                .ok()
-                .map(|vote_state_update| (VoteTransaction::from(vote_state_update), Some(hash)))
+        VoteInstruction::CompactUpdateVoteStateSwitch(vote_state_update, hash) => {
+            Some((VoteTransaction::from(vote_state_update), Some(hash)))
         }
         VoteInstruction::Authorize(_, _)
         | VoteInstruction::AuthorizeChecked(_)

diff --git a/sdk/program/src/vote/instruction.rs b/sdk/program/src/vote/instruction.rs
@@ -9,8 +9,9 @@ use {
         vote::{
             program::id,
             state::{
-                CompactVoteStateUpdate, Vote, VoteAuthorize, VoteAuthorizeCheckedWithSeedArgs,
-                VoteAuthorizeWithSeedArgs, VoteInit, VoteState, VoteStateUpdate,
+                serde_compact_vote_state_update, Vote, VoteAuthorize,
+                VoteAuthorizeCheckedWithSeedArgs, VoteAuthorizeWithSeedArgs, VoteInit, VoteState,
+                VoteStateUpdate,
             },
         },
     },
@@ -132,14 +133,18 @@ pub enum VoteInstruction {
     /// # Account references
     ///   0. `[Write]` Vote account to vote with
     ///   1. `[SIGNER]` Vote authority
-    CompactUpdateVoteState(CompactVoteStateUpdate),
+    #[serde(with = "serde_compact_vote_state_update")]
+    CompactUpdateVoteState(VoteStateUpdate),
 
     /// Update the onchain vote state for the signer along with a switching proof.
     ///
     /// # Account references
     ///   0. `[Write]` Vote account to vote with
     ///   1. `[SIGNER]` Vote authority
-    CompactUpdateVoteStateSwitch(CompactVoteStateUpdate, Hash),
+    CompactUpdateVoteStateSwitch(
+        #[serde(with = "serde_compact_vote_state_update")] VoteStateUpdate,
+        Hash,
+    ),
 }
 
 fn initialize_account(vote_pubkey: &Pubkey, vote_init: &VoteInit) -> Instruction {
@@ -387,7 +392,7 @@ pub fn update_vote_state_switch(
 pub fn compact_update_vote_state(
     vote_pubkey: &Pubkey,
     authorized_voter_pubkey: &Pubkey,
-    compact_vote_state_update: CompactVoteStateUpdate,
+    vote_state_update: VoteStateUpdate,
 ) -> Instruction {
     let account_metas = vec![
         AccountMeta::new(*vote_pubkey, false),
@@ -396,15 +401,15 @@ pub fn compact_update_vote_state(
 
     Instruction::new_with_bincode(
         id(),
-        &VoteInstruction::CompactUpdateVoteState(compact_vote_state_update),
+        &VoteInstruction::CompactUpdateVoteState(vote_state_update),
         account_metas,
     )
 }
 
 pub fn compact_update_vote_state_switch(
     vote_pubkey: &Pubkey,
     authorized_voter_pubkey: &Pubkey,
-    vote_state_update: CompactVoteStateUpdate,
+    vote_state_update: VoteStateUpdate,
     proof_hash: Hash,
 ) -> Instruction {
     let account_metas = vec![