From 8fe05e701e6b490c959304174d1df900ed528a97 Mon Sep 17 00:00:00 2001 From: Ashwin Sekar Date: Tue, 1 Oct 2024 14:56:28 +0000 Subject: [PATCH] pr feedback: yolo_hash -> vote_only_hash, use option for replay_slot --- core/src/consensus.rs | 2 +- core/src/consensus/tower1_14_11.rs | 2 +- cost-model/src/transaction_cost.rs | 2 +- local-cluster/tests/local_cluster.rs | 2 +- programs/vote/benches/process_vote.rs | 4 +- programs/vote/src/vote_state/mod.rs | 66 +++++++++++++-------------- sdk/program/src/vote/instruction.rs | 4 +- sdk/program/src/vote/state/mod.rs | 65 +++++++++++++------------- transaction-status/src/parse_vote.rs | 16 +++++-- vote/src/vote_transaction.rs | 2 +- wen-restart/src/wen_restart.rs | 2 +- 11 files changed, 89 insertions(+), 78 deletions(-) diff --git a/core/src/consensus.rs b/core/src/consensus.rs index e1a6bfd179de91..dee9b0ee4f2f07 100644 --- a/core/src/consensus.rs +++ b/core/src/consensus.rs @@ -239,7 +239,7 @@ pub(crate) enum BlockhashStatus { #[cfg_attr( feature = "frozen-abi", derive(AbiExample), - frozen_abi(digest = "Lbx8JWmgAWq4SSo1XyrXprAgPtp9ncRNJJPCKH1pZ3W") + frozen_abi(digest = "jq28RP4SJpNQ3tZJ88zUcrEShMzhiTrNajyTXHgE7vR") )] #[derive(Clone, Serialize, Deserialize, Debug, PartialEq)] pub struct Tower { diff --git a/core/src/consensus/tower1_14_11.rs b/core/src/consensus/tower1_14_11.rs index a0c20fb8880694..2a26fb183d6ceb 100644 --- a/core/src/consensus/tower1_14_11.rs +++ b/core/src/consensus/tower1_14_11.rs @@ -9,7 +9,7 @@ use { #[cfg_attr( feature = "frozen-abi", derive(AbiExample), - frozen_abi(digest = "FYKsZaj6MH3hqA4ksJWEG8LHQZF3Ty5PQgfbexnpAPtB") + frozen_abi(digest = "FiF3sSBJQFoKAcU4xz3vpSopBqu1XfS7GE9SxNHaTzCq") )] #[derive(Clone, Serialize, Deserialize, Debug, PartialEq)] pub struct Tower1_14_11 { diff --git a/cost-model/src/transaction_cost.rs b/cost-model/src/transaction_cost.rs index 9d7369b5ee4103..8765f7fc096882 100644 --- a/cost-model/src/transaction_cost.rs +++ b/cost-model/src/transaction_cost.rs @@ -248,7 +248,7 @@ mod tests { // expected vote tx cost: 2 write locks, 1 sig, 1 vote ix, 8cu of loaded accounts size, let expected_vote_cost = SIMPLE_VOTE_USAGE_COST; // expected non-vote tx cost would include default loaded accounts size cost (16384) additionally - let expected_none_vote_cost = 20553; + let expected_none_vote_cost = 20551; let vote_cost = CostModel::calculate_cost(&vote_transaction, &FeatureSet::all_enabled()); let none_vote_cost = diff --git a/local-cluster/tests/local_cluster.rs b/local-cluster/tests/local_cluster.rs index a4c767e22ede52..3e950a44891b51 100644 --- a/local-cluster/tests/local_cluster.rs +++ b/local-cluster/tests/local_cluster.rs @@ -3967,7 +3967,7 @@ fn run_duplicate_shreds_broadcast_leader(vote_on_duplicate: bool) { AncestorIterator::new_inclusive(latest_vote_slot, &leader_blockstore) .nth(MAX_LOCKOUT_HISTORY); vote.root = root; - vote.hash = vote_hash; + vote.replay_hash = vote_hash; let vote_tx = vote_transaction::new_tower_sync_transaction( vote, leader_vote_tx.message.recent_blockhash, diff --git a/programs/vote/benches/process_vote.rs b/programs/vote/benches/process_vote.rs index 2e19923597fb53..422ec5f646e7f3 100644 --- a/programs/vote/benches/process_vote.rs +++ b/programs/vote/benches/process_vote.rs @@ -192,7 +192,9 @@ fn bench_process_tower_sync(bencher: &mut Bencher) { ((num_initial_votes.saturating_add(1)..=last_vote_slot).zip((1u32..=31).rev())).collect(); let mut tower_sync = TowerSync::from(slots_and_lockouts); tower_sync.root = Some(num_initial_votes); - tower_sync.hash = last_vote_hash; + tower_sync.vote_only_hash = Hash::default(); + tower_sync.replay_slot = Some(last_vote_slot); + tower_sync.replay_hash = last_vote_hash; tower_sync.block_id = Hash::new_unique(); let instruction_data = bincode::serialize(&VoteInstruction::TowerSync(tower_sync)).unwrap(); diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index ff46ab9de7c60d..46b849df0e73ec 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -30,7 +30,7 @@ use { #[cfg_attr( feature = "frozen-abi", derive(AbiExample, AbiEnumVisitor), - frozen_abi(digest = "5zYYafKxFiaQj4s1PUhK7bW2rKAXBBcnpHVxR8kfpZhQ") + frozen_abi(digest = "2ur9qpyZBfJBTLncyqwh9pyQR4U8Xgd2iyMJPPKoSC2f") )] #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] pub enum VoteTransaction { @@ -90,7 +90,7 @@ impl VoteTransaction { VoteTransaction::Vote(vote) => vote.hash, VoteTransaction::VoteStateUpdate(vote_state_update) => vote_state_update.hash, VoteTransaction::CompactVoteStateUpdate(vote_state_update) => vote_state_update.hash, - VoteTransaction::TowerSync(tower_sync) => tower_sync.hash, + VoteTransaction::TowerSync(tower_sync) => tower_sync.replay_hash, } } @@ -1176,7 +1176,7 @@ fn do_process_tower_sync( vote_state, &mut tower_sync.lockouts, &mut tower_sync.root, - tower_sync.hash, + tower_sync.replay_hash, slot_hashes, )?; process_new_vote_state( @@ -3178,7 +3178,7 @@ mod tests { &empty_vote_state, &mut tower_sync.lockouts, &mut tower_sync.root, - tower_sync.hash, + tower_sync.replay_hash, &empty_slot_hashes ), Err(VoteError::EmptySlots), @@ -3191,7 +3191,7 @@ mod tests { &empty_vote_state, &mut tower_sync.lockouts, &mut tower_sync.root, - tower_sync.hash, + tower_sync.replay_hash, &empty_slot_hashes ), Err(VoteError::SlotsMismatch), @@ -3212,7 +3212,7 @@ mod tests { &vote_state, &mut tower_sync.lockouts, &mut tower_sync.root, - tower_sync.hash, + tower_sync.replay_hash, &slot_hashes ), Err(VoteError::VoteTooOld), @@ -3229,7 +3229,7 @@ mod tests { &vote_state, &mut tower_sync.lockouts, &mut tower_sync.root, - tower_sync.hash, + tower_sync.replay_hash, &slot_hashes ), Err(VoteError::VoteTooOld), @@ -3279,13 +3279,13 @@ mod tests { // Root slot in the `TowerSync` should be updated to match the root slot in the // current vote state let mut tower_sync = TowerSync::from(proposed_slots_and_lockouts); - tower_sync.hash = proposed_hash; + tower_sync.replay_hash = proposed_hash; tower_sync.root = Some(proposed_root); check_and_filter_proposed_vote_state( &vote_state, &mut tower_sync.lockouts, &mut tower_sync.root, - tower_sync.hash, + tower_sync.replay_hash, &slot_hashes, ) .unwrap(); @@ -3451,13 +3451,13 @@ mod tests { .unwrap() .1; let mut tower_sync = TowerSync::from(vec![(2, 2), (1, 3), (vote_slot, 1)]); - tower_sync.hash = vote_slot_hash; + tower_sync.replay_hash = vote_slot_hash; assert_eq!( check_and_filter_proposed_vote_state( &vote_state, &mut tower_sync.lockouts, &mut tower_sync.root, - tower_sync.hash, + tower_sync.replay_hash, &slot_hashes ), Err(VoteError::SlotsNotOrdered), @@ -3465,13 +3465,13 @@ mod tests { // Test with a `TowerSync` where there are multiples of the same slot let mut tower_sync = TowerSync::from(vec![(2, 2), (2, 2), (vote_slot, 1)]); - tower_sync.hash = vote_slot_hash; + tower_sync.replay_hash = vote_slot_hash; assert_eq!( check_and_filter_proposed_vote_state( &vote_state, &mut tower_sync.lockouts, &mut tower_sync.root, - tower_sync.hash, + tower_sync.replay_hash, &slot_hashes ), Err(VoteError::SlotsNotOrdered), @@ -3501,12 +3501,12 @@ mod tests { (missing_older_than_history_slot, 2), (vote_slot, 3), ]); - tower_sync.hash = vote_slot_hash; + tower_sync.replay_hash = vote_slot_hash; check_and_filter_proposed_vote_state( &vote_state, &mut tower_sync.lockouts, &mut tower_sync.root, - tower_sync.hash, + tower_sync.replay_hash, &slot_hashes, ) .unwrap(); @@ -3554,12 +3554,12 @@ mod tests { let existing_older_than_history_slot = 4; let mut tower_sync = TowerSync::from(vec![(existing_older_than_history_slot, 3), (vote_slot, 2)]); - tower_sync.hash = vote_slot_hash; + tower_sync.replay_hash = vote_slot_hash; check_and_filter_proposed_vote_state( &vote_state, &mut tower_sync.lockouts, &mut tower_sync.root, - tower_sync.hash, + tower_sync.replay_hash, &slot_hashes, ) .unwrap(); @@ -3621,12 +3621,12 @@ mod tests { (12, 2), (vote_slot, 1), ]); - tower_sync.hash = vote_slot_hash; + tower_sync.replay_hash = vote_slot_hash; check_and_filter_proposed_vote_state( &vote_state, &mut tower_sync.lockouts, &mut tower_sync.root, - tower_sync.hash, + tower_sync.replay_hash, &slot_hashes, ) .unwrap(); @@ -3675,13 +3675,13 @@ mod tests { .unwrap() .1; let mut tower_sync = TowerSync::from(vec![(missing_vote_slot, 2), (vote_slot, 3)]); - tower_sync.hash = vote_slot_hash; + tower_sync.replay_hash = vote_slot_hash; assert_eq!( check_and_filter_proposed_vote_state( &vote_state, &mut tower_sync.lockouts, &mut tower_sync.root, - tower_sync.hash, + tower_sync.replay_hash, &slot_hashes ), Err(VoteError::SlotsMismatch), @@ -3696,13 +3696,13 @@ mod tests { (missing_vote_slot, 2), (vote_slot, 1), ]); - tower_sync.hash = vote_slot_hash; + tower_sync.replay_hash = vote_slot_hash; assert_eq!( check_and_filter_proposed_vote_state( &vote_state, &mut tower_sync.lockouts, &mut tower_sync.root, - tower_sync.hash, + tower_sync.replay_hash, &slot_hashes ), Err(VoteError::SlotsMismatch), @@ -3731,14 +3731,14 @@ mod tests { .unwrap() .1; let mut tower_sync = TowerSync::from(vec![(vote_slot, 1)]); - tower_sync.hash = vote_slot_hash; + tower_sync.replay_hash = vote_slot_hash; tower_sync.root = Some(new_root); assert_eq!( check_and_filter_proposed_vote_state( &vote_state, &mut tower_sync.lockouts, &mut tower_sync.root, - tower_sync.hash, + tower_sync.replay_hash, &slot_hashes ), Err(VoteError::RootOnDifferentFork), @@ -3758,13 +3758,13 @@ mod tests { let missing_vote_slot = slot_hashes.first().unwrap().0 + 1; let vote_slot_hash = Hash::new_unique(); let mut tower_sync = TowerSync::from(vec![(8, 2), (missing_vote_slot, 3)]); - tower_sync.hash = vote_slot_hash; + tower_sync.replay_hash = vote_slot_hash; assert_eq!( check_and_filter_proposed_vote_state( &vote_state, &mut tower_sync.lockouts, &mut tower_sync.root, - tower_sync.hash, + tower_sync.replay_hash, &slot_hashes ), Err(VoteError::SlotsMismatch), @@ -3788,12 +3788,12 @@ mod tests { .unwrap() .1; let mut tower_sync = TowerSync::from(vec![(2, 4), (4, 3), (6, 2), (vote_slot, 1)]); - tower_sync.hash = vote_slot_hash; + tower_sync.replay_hash = vote_slot_hash; check_and_filter_proposed_vote_state( &vote_state, &mut tower_sync.lockouts, &mut tower_sync.root, - tower_sync.hash, + tower_sync.replay_hash, &slot_hashes, ) .unwrap(); @@ -3841,12 +3841,12 @@ mod tests { .unwrap() .1; let mut tower_sync = TowerSync::from(vec![(4, 2), (vote_slot, 1)]); - tower_sync.hash = vote_slot_hash; + tower_sync.replay_hash = vote_slot_hash; check_and_filter_proposed_vote_state( &vote_state, &mut tower_sync.lockouts, &mut tower_sync.root, - tower_sync.hash, + tower_sync.replay_hash, &slot_hashes, ) .unwrap(); @@ -3892,13 +3892,13 @@ mod tests { let vote_slot = vote_state.votes.back().unwrap().slot() + 2; let vote_slot_hash = Hash::new_unique(); let mut tower_sync = TowerSync::from(vec![(2, 4), (4, 3), (6, 2), (vote_slot, 1)]); - tower_sync.hash = vote_slot_hash; + tower_sync.replay_hash = vote_slot_hash; assert_eq!( check_and_filter_proposed_vote_state( &vote_state, &mut tower_sync.lockouts, &mut tower_sync.root, - tower_sync.hash, + tower_sync.replay_hash, &slot_hashes, ), Err(VoteError::SlotHashMismatch), diff --git a/sdk/program/src/vote/instruction.rs b/sdk/program/src/vote/instruction.rs index e707c9e06d05bd..8ce6f32cb7567f 100644 --- a/sdk/program/src/vote/instruction.rs +++ b/sdk/program/src/vote/instruction.rs @@ -218,7 +218,9 @@ impl VoteInstruction { | Self::UpdateVoteStateSwitch(vote_state_update, _) | Self::CompactUpdateVoteState(vote_state_update) | Self::CompactUpdateVoteStateSwitch(vote_state_update, _) => vote_state_update.hash, - Self::TowerSync(tower_sync) | Self::TowerSyncSwitch(tower_sync, _) => tower_sync.hash, + Self::TowerSync(tower_sync) | Self::TowerSyncSwitch(tower_sync, _) => { + tower_sync.replay_hash + } _ => panic!("Tried to get hash on non simple vote instruction"), } } diff --git a/sdk/program/src/vote/state/mod.rs b/sdk/program/src/vote/state/mod.rs index 443df0c352beed..6c24fcd6f86be9 100644 --- a/sdk/program/src/vote/state/mod.rs +++ b/sdk/program/src/vote/state/mod.rs @@ -221,7 +221,7 @@ impl VoteStateUpdate { #[cfg_attr( feature = "frozen-abi", - frozen_abi(digest = "DqMoZ6hSsG1wguKx49kjZmU8sEu7jjzDWqg14qxoeinM"), + frozen_abi(digest = "9WcxsxCu6sbGM2nsASZKRjVTiffH6y8UZtTKDPud5WMV"), derive(AbiExample) )] #[derive(Serialize, Default, Deserialize, Debug, PartialEq, Eq, Clone)] @@ -231,12 +231,12 @@ pub struct TowerSync { /// the proposed root pub root: Option, /// signature of the VoteState's at the latest slot in `lockouts` - pub yolo_hash: Hash, + pub vote_only_hash: Hash, /// the latest fully replayed slot. until asynchronous execution /// is active this will be equal to the latest slot in `lockouts` - pub replay_tip: Slot, - /// signature of the bank's state at `replay_tip` - pub hash: Hash, + pub replay_slot: Option, + /// signature of the bank's state at `replay_slot` + pub replay_hash: Hash, /// processing timestamp of last slot pub timestamp: Option, /// the unique identifier for the chain up to and @@ -256,9 +256,9 @@ impl From> for TowerSync { Self { lockouts, root: None, - yolo_hash: Hash::default(), - replay_tip: 0, - hash: Hash::default(), + vote_only_hash: Hash::default(), + replay_slot: None, + replay_hash: Hash::default(), timestamp: None, block_id: Hash::default(), } @@ -269,17 +269,16 @@ impl TowerSync { pub fn new( lockouts: VecDeque, root: Option, - hash: Hash, + replay_hash: Hash, block_id: Hash, ) -> Self { - let replay_tip = lockouts.back().map(|l| l.slot).unwrap_or(0); - let yolo_hash = hash; + let replay_slot = lockouts.back().map(|l| l.slot); Self { lockouts, root, - yolo_hash, - replay_tip, - hash, + vote_only_hash: Hash::default(), + replay_slot, + replay_hash, timestamp: None, block_id, } @@ -288,20 +287,20 @@ impl TowerSync { /// Creates a tower with consecutive votes for `slot - MAX_LOCKOUT_HISTORY + 1` to `slot` inclusive. /// If `slot >= MAX_LOCKOUT_HISTORY`, sets the root to `(slot - MAX_LOCKOUT_HISTORY)` /// Sets the hash to `hash` and leaves `block_id` unset. - pub fn new_from_slot(slot: Slot, hash: Hash) -> Self { + pub fn new_from_slot(slot: Slot, replay_hash: Hash) -> Self { let lowest_slot = slot .saturating_add(1) .saturating_sub(MAX_LOCKOUT_HISTORY as u64); let slots: Vec<_> = (lowest_slot..slot.saturating_add(1)).collect(); Self::new_from_slots( slots, - hash, + replay_hash, (lowest_slot > 0).then(|| lowest_slot.saturating_sub(1)), ) } /// Creates a tower with consecutive confirmation for `slots` - pub fn new_from_slots(slots: Vec, hash: Hash, root: Option) -> Self { + pub fn new_from_slots(slots: Vec, replay_hash: Hash, root: Option) -> Self { let lockouts: VecDeque = slots .into_iter() .rev() @@ -309,12 +308,12 @@ impl TowerSync { .map(|(cc, s)| Lockout::new_with_confirmation_count(s, cc.saturating_add(1) as u32)) .rev() .collect(); - let replay_tip = lockouts.back().map(|l| l.slot).unwrap_or(0); + let replay_slot = lockouts.back().map(|l| l.slot); Self { lockouts, - replay_tip, - yolo_hash: hash, - hash, + replay_slot, + vote_only_hash: Hash::default(), + replay_hash, root, timestamp: None, block_id: Hash::default(), @@ -1104,9 +1103,9 @@ pub mod serde_tower_sync { root: Slot, #[serde(with = "short_vec")] lockout_offsets: Vec, - replay_tip: Slot, - yolo_hash: Hash, - hash: Hash, + vote_only_hash: Hash, + replay_slot: Option, + replay_hash: Hash, timestamp: Option, block_id: Hash, } @@ -1135,9 +1134,9 @@ pub mod serde_tower_sync { let compact_tower_sync = CompactTowerSync { root: tower_sync.root.unwrap_or(Slot::MAX), lockout_offsets: lockout_offsets.collect::>()?, - replay_tip: tower_sync.replay_tip, - yolo_hash: tower_sync.yolo_hash, - hash: tower_sync.hash, + vote_only_hash: tower_sync.vote_only_hash, + replay_slot: tower_sync.replay_slot, + replay_hash: tower_sync.replay_hash, timestamp: tower_sync.timestamp, block_id: tower_sync.block_id, }; @@ -1151,9 +1150,9 @@ pub mod serde_tower_sync { let CompactTowerSync { root, lockout_offsets, - replay_tip, - yolo_hash, - hash, + vote_only_hash, + replay_slot, + replay_hash, timestamp, block_id, } = CompactTowerSync::deserialize(deserializer)?; @@ -1177,9 +1176,9 @@ pub mod serde_tower_sync { Ok(TowerSync { root, lockouts: lockouts.collect::>()?, - replay_tip, - yolo_hash, - hash, + vote_only_hash, + replay_slot, + replay_hash, timestamp, block_id, }) diff --git a/transaction-status/src/parse_vote.rs b/transaction-status/src/parse_vote.rs index 7276a43f75d096..3b0886177bf099 100644 --- a/transaction-status/src/parse_vote.rs +++ b/transaction-status/src/parse_vote.rs @@ -176,7 +176,9 @@ pub fn parse_vote( let tower_sync = json!({ "lockouts": tower_sync.lockouts, "root": tower_sync.root, - "hash": tower_sync.hash.to_string(), + "vote_only_hash": tower_sync.vote_only_hash.to_string(), + "replay_slot": tower_sync.replay_slot, + "replay_hash": tower_sync.replay_hash.to_string(), "timestamp": tower_sync.timestamp, "blockId": tower_sync.block_id.to_string(), }); @@ -194,7 +196,9 @@ pub fn parse_vote( let tower_sync = json!({ "lockouts": tower_sync.lockouts, "root": tower_sync.root, - "hash": tower_sync.hash.to_string(), + "vote_only_hash": tower_sync.vote_only_hash.to_string(), + "replay_slot": tower_sync.replay_slot, + "replay_hash": tower_sync.replay_hash.to_string(), "timestamp": tower_sync.timestamp, "blockId": tower_sync.block_id.to_string(), }); @@ -925,7 +929,9 @@ mod test { "towerSync": { "lockouts": tower_sync.lockouts, "root": None::, - "hash": Hash::default().to_string(), + "vote_only_hash": Hash::default().to_string(), + "replay_slot": None::, + "replay_hash": Hash::default().to_string(), "timestamp": None::, "blockId": Hash::default().to_string(), }, @@ -970,7 +976,9 @@ mod test { "towerSync": { "lockouts": tower_sync.lockouts, "root": None::, - "hash": Hash::default().to_string(), + "vote_only_hash": Hash::default().to_string(), + "replay_slot": None::, + "replay_hash": Hash::default().to_string(), "timestamp": None::, "blockId": Hash::default().to_string(), }, diff --git a/vote/src/vote_transaction.rs b/vote/src/vote_transaction.rs index a5b0d382d6c436..2c8a0eb3dfd6cf 100644 --- a/vote/src/vote_transaction.rs +++ b/vote/src/vote_transaction.rs @@ -38,7 +38,7 @@ impl VoteTransaction { match self { VoteTransaction::Vote(vote) => vote.hash, VoteTransaction::VoteStateUpdate(vote_state_update) => vote_state_update.hash, - VoteTransaction::TowerSync(tower_sync) => tower_sync.hash, + VoteTransaction::TowerSync(tower_sync) => tower_sync.replay_hash, } } diff --git a/wen-restart/src/wen_restart.rs b/wen-restart/src/wen_restart.rs index 924debb2adf226..d5c633c6276054 100644 --- a/wen-restart/src/wen_restart.rs +++ b/wen-restart/src/wen_restart.rs @@ -2120,7 +2120,7 @@ mod tests { assert!(remove_file(&test_state.wen_restart_proto_path).is_ok()); // Test the case where the file is not found. let mut vote = TowerSync::from(vec![(test_state.last_voted_fork_slots[0], 1)]); - vote.hash = last_vote_bankhash; + vote.replay_hash = last_vote_bankhash; let last_vote = VoteTransaction::from(vote); assert_eq!( initialize(