diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index 1796ae2b64..6fda2e0ab4 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -1970,8 +1970,7 @@ impl ReplayStage { } if my_latest_landed_vote >= last_voted_slot || heaviest_bank_on_same_fork - .check_hash_age(&tower.last_vote_tx_blockhash(), MAX_PROCESSING_AGE) - .unwrap_or(false) + .is_hash_valid_for_age(&tower.last_vote_tx_blockhash(), MAX_PROCESSING_AGE) // In order to avoid voting on multiple forks all past MAX_PROCESSING_AGE that don't // include the last voted blockhash || last_vote_refresh_time.last_refresh_time.elapsed().as_millis() < MAX_VOTE_REFRESH_INTERVAL_MILLIS as u128 diff --git a/poh/src/poh_recorder.rs b/poh/src/poh_recorder.rs index 1163355a86..b54ee0a9e2 100644 --- a/poh/src/poh_recorder.rs +++ b/poh/src/poh_recorder.rs @@ -1967,7 +1967,7 @@ mod tests { poh_recorder.tick(); } poh_recorder.set_bank(&bank); - assert_eq!(Some(false), bank.check_hash_age(&genesis_hash, 1)); + assert!(!bank.is_hash_valid_for_age(&genesis_hash, 1)); } } diff --git a/runtime/benches/bank.rs b/runtime/benches/bank.rs index d8b4af8e54..38cf6bde58 100644 --- a/runtime/benches/bank.rs +++ b/runtime/benches/bank.rs @@ -203,10 +203,7 @@ fn bench_bank_update_recent_blockhashes(bencher: &mut Bencher) { goto_end_of_slot(Arc::get_mut(&mut bank).unwrap()); } // Verify blockhash_queue is full (genesis hash has been kicked out) - assert_eq!( - Some(false), - bank.check_hash_age(&genesis_hash, MAX_RECENT_BLOCKHASHES) - ); + assert!(!bank.is_hash_valid_for_age(&genesis_hash, MAX_RECENT_BLOCKHASHES)); bencher.iter(|| { bank.update_recent_blockhashes(); }); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index f8b520e954..439142e05c 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3332,7 +3332,7 @@ impl Bank { pub fn is_blockhash_valid(&self, hash: &Hash) -> bool { let blockhash_queue = self.blockhash_queue.read().unwrap(); - blockhash_queue.check_hash(hash) + blockhash_queue.is_hash_valid(hash) } pub fn get_minimum_balance_for_rent_exemption(&self, data_len: usize) -> u64 { @@ -3680,14 +3680,10 @@ impl Bank { .map(|(tx, lock_res)| match lock_res { Ok(()) => { let recent_blockhash = tx.message().recent_blockhash(); - let hash_age = hash_queue.check_hash_age(recent_blockhash, max_age); - if hash_age == Some(true) { + if hash_queue.is_hash_valid_for_age(recent_blockhash, max_age) { (Ok(()), None) } else if let Some((address, account)) = self.check_transaction_for_nonce(tx) { (Ok(()), Some(NoncePartial::new(address, account))) - } else if hash_age == Some(false) { - error_counters.blockhash_too_old += 1; - (Err(TransactionError::BlockhashNotFound), None) } else { error_counters.blockhash_not_found += 1; (Err(TransactionError::BlockhashNotFound), None) @@ -3737,11 +3733,11 @@ impl Bank { self.blockhash_queue.read().unwrap().get_hash_age(hash) } - pub fn check_hash_age(&self, hash: &Hash, max_age: usize) -> Option { + pub fn is_hash_valid_for_age(&self, hash: &Hash, max_age: usize) -> bool { self.blockhash_queue .read() .unwrap() - .check_hash_age(hash, max_age) + .is_hash_valid_for_age(hash, max_age) } fn check_message_for_nonce(&self, message: &SanitizedMessage) -> Option { @@ -11592,7 +11588,7 @@ pub(crate) mod tests { assert_eq!(recent_blockhashes.len(), i); let most_recent_hash = recent_blockhashes.iter().next().unwrap().blockhash; // Check order - assert_eq!(Some(true), bank.check_hash_age(&most_recent_hash, 0)); + assert!(bank.is_hash_valid_for_age(&most_recent_hash, 0)); goto_end_of_slot(Arc::get_mut(&mut bank).unwrap()); bank = Arc::new(new_from_parent(&bank)); } diff --git a/runtime/src/blockhash_queue.rs b/runtime/src/blockhash_queue.rs index f3247c4ef0..ae24183470 100644 --- a/runtime/src/blockhash_queue.rs +++ b/runtime/src/blockhash_queue.rs @@ -11,16 +11,16 @@ use { #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize, AbiExample)] struct HashAge { fee_calculator: FeeCalculator, - hash_height: u64, + hash_index: u64, timestamp: u64, } /// Low memory overhead, so can be cloned for every checkpoint -#[frozen_abi(digest = "J1fGiMHyiKEBcWE6mfm7grAEGJgYEaVLzcrNZvd37iA2")] +#[frozen_abi(digest = "J66ssCYGtWdQu5oyJxFKFeZY86nUjThBdBeXQYuRPDvE")] #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, AbiExample)] pub struct BlockhashQueue { - /// updated whenever an hash is registered - hash_height: u64, + /// index of last hash to be registered + last_hash_index: u64, /// last hash to be registered last_hash: Option, @@ -41,17 +41,12 @@ impl BlockhashQueue { pub fn new(max_age: usize) -> Self { Self { ages: HashMap::new(), - hash_height: 0, + last_hash_index: 0, last_hash: None, max_age, } } - #[allow(dead_code)] - pub fn hash_height(&self) -> u64 { - self.hash_height - } - pub fn last_hash(&self) -> Hash { self.last_hash.expect("no hash has been set") } @@ -62,24 +57,23 @@ impl BlockhashQueue { .map(|hash_age| hash_age.fee_calculator.lamports_per_signature) } - /// Check if the age of the hash is within the max_age - /// return false for any hashes with an age above max_age - /// return None for any hashes that were not found - pub fn check_hash_age(&self, hash: &Hash, max_age: usize) -> Option { + /// Check if the age of the hash is within the queue's max age + pub fn is_hash_valid(&self, hash: &Hash) -> bool { + self.ages.get(hash).is_some() + } + + /// Check if the age of the hash is within the specified age + pub fn is_hash_valid_for_age(&self, hash: &Hash, max_age: usize) -> bool { self.ages .get(hash) - .map(|age| self.hash_height - age.hash_height <= max_age as u64) + .map(|age| Self::is_hash_index_valid(self.last_hash_index, max_age, age.hash_index)) + .unwrap_or(false) } pub fn get_hash_age(&self, hash: &Hash) -> Option { self.ages .get(hash) - .map(|age| self.hash_height - age.hash_height) - } - - /// check if hash is valid - pub fn check_hash(&self, hash: &Hash) -> bool { - self.ages.get(hash).is_some() + .map(|age| self.last_hash_index - age.hash_index) } pub fn genesis_hash(&mut self, hash: &Hash, lamports_per_signature: u64) { @@ -87,7 +81,7 @@ impl BlockhashQueue { *hash, HashAge { fee_calculator: FeeCalculator::new(lamports_per_signature), - hash_height: 0, + hash_index: 0, timestamp: timestamp(), }, ); @@ -95,26 +89,23 @@ impl BlockhashQueue { self.last_hash = Some(*hash); } - fn check_age(hash_height: u64, max_age: usize, age: &HashAge) -> bool { - hash_height - age.hash_height <= max_age as u64 + fn is_hash_index_valid(last_hash_index: u64, max_age: usize, hash_index: u64) -> bool { + last_hash_index - hash_index <= max_age as u64 } pub fn register_hash(&mut self, hash: &Hash, lamports_per_signature: u64) { - self.hash_height += 1; - let hash_height = self.hash_height; - - // this clean up can be deferred until sigs gets larger - // because we verify age.nth every place we check for validity - let max_age = self.max_age; - if self.ages.len() >= max_age { - self.ages - .retain(|_, age| Self::check_age(hash_height, max_age, age)); + self.last_hash_index += 1; + if self.ages.len() >= self.max_age { + self.ages.retain(|_, age| { + Self::is_hash_index_valid(self.last_hash_index, self.max_age, age.hash_index) + }); } + self.ages.insert( *hash, HashAge { fee_calculator: FeeCalculator::new(lamports_per_signature), - hash_height, + hash_index: self.last_hash_index, timestamp: timestamp(), }, ); @@ -122,16 +113,6 @@ impl BlockhashQueue { self.last_hash = Some(*hash); } - /// Maps a hash height to a timestamp - pub fn hash_height_to_timestamp(&self, hash_height: u64) -> Option { - for age in self.ages.values() { - if age.hash_height == hash_height { - return Some(age.timestamp); - } - } - None - } - #[deprecated( since = "1.9.0", note = "Please do not use, will no longer be available in the future" @@ -139,7 +120,7 @@ impl BlockhashQueue { #[allow(deprecated)] pub fn get_recent_blockhashes(&self) -> impl Iterator { (self.ages).iter().map(|(k, v)| { - recent_blockhashes::IterItem(v.hash_height, k, v.fee_calculator.lamports_per_signature) + recent_blockhashes::IterItem(v.hash_index, k, v.fee_calculator.lamports_per_signature) }) } @@ -161,10 +142,10 @@ mod tests { fn test_register_hash() { let last_hash = Hash::default(); let mut hash_queue = BlockhashQueue::new(100); - assert!(!hash_queue.check_hash(&last_hash)); + assert!(!hash_queue.is_hash_valid(&last_hash)); hash_queue.register_hash(&last_hash, 0); - assert!(hash_queue.check_hash(&last_hash)); - assert_eq!(hash_queue.hash_height(), 1); + assert!(hash_queue.is_hash_valid(&last_hash)); + assert_eq!(hash_queue.last_hash_index, 1); } #[test] @@ -176,13 +157,13 @@ mod tests { hash_queue.register_hash(&last_hash, 0); } // Assert we're no longer able to use the oldest hash. - assert!(!hash_queue.check_hash(&last_hash)); - assert_eq!(None, hash_queue.check_hash_age(&last_hash, 0)); + assert!(!hash_queue.is_hash_valid(&last_hash)); + assert!(!hash_queue.is_hash_valid_for_age(&last_hash, 0)); // Assert we are not able to use the oldest remaining hash. let last_valid_hash = hash(&serialize(&1).unwrap()); - assert!(hash_queue.check_hash(&last_valid_hash)); - assert_eq!(Some(false), hash_queue.check_hash_age(&last_valid_hash, 0)); + assert!(hash_queue.is_hash_valid(&last_valid_hash)); + assert!(!hash_queue.is_hash_valid_for_age(&last_valid_hash, 0)); } /// test that when max age is 0, that a valid last_hash still passes the age check @@ -192,7 +173,7 @@ mod tests { let mut hash_queue = BlockhashQueue::new(100); hash_queue.register_hash(&last_hash, 0); assert_eq!(last_hash, hash_queue.last_hash()); - assert_eq!(Some(true), hash_queue.check_hash_age(&last_hash, 0)); + assert!(hash_queue.is_hash_valid_for_age(&last_hash, 0)); } #[test] @@ -211,10 +192,7 @@ mod tests { // Verify that the returned hashes are most recent #[allow(deprecated)] for IterItem(_slot, hash, _lamports_per_signature) in recent_blockhashes { - assert_eq!( - Some(true), - blockhash_queue.check_hash_age(hash, MAX_RECENT_BLOCKHASHES) - ); + assert!(blockhash_queue.is_hash_valid_for_age(hash, MAX_RECENT_BLOCKHASHES)); } } @@ -268,14 +246,14 @@ mod tests { } #[test] - fn test_check_hash_age() { + fn test_is_hash_valid_for_age() { const MAX_AGE: usize = 10; let mut hash_list: Vec = Vec::new(); hash_list.resize_with(MAX_AGE + 1, Hash::new_unique); let mut hash_queue = BlockhashQueue::new(MAX_AGE); for hash in &hash_list { - assert!(hash_queue.check_hash_age(hash, MAX_AGE).is_none()); + assert!(!hash_queue.is_hash_valid_for_age(hash, MAX_AGE)); } for hash in &hash_list { @@ -286,19 +264,11 @@ mod tests { // the age of a hash is within max age, the hash from 11 slots ago is considered // to be within the max age of 10. for hash in &hash_list { - assert_eq!(hash_queue.check_hash_age(hash, MAX_AGE), Some(true)); + assert!(hash_queue.is_hash_valid_for_age(hash, MAX_AGE)); } - // When max age is 0, the most recent blockhash is still considered valid - assert_eq!( - hash_queue.check_hash_age(&hash_list[MAX_AGE], 0), - Some(true) - ); - - // Just because a hash is in the queue, doesn't mean it's valid for max age - assert_eq!( - hash_queue.check_hash_age(&hash_list[0], MAX_AGE - 1), - Some(false) - ); + // When max age is 0, only the most recent blockhash is still considered valid + assert!(hash_queue.is_hash_valid_for_age(&hash_list[MAX_AGE], 0)); + assert!(!hash_queue.is_hash_valid_for_age(&hash_list[MAX_AGE - 1], 0)); } } diff --git a/runtime/src/serde_snapshot/tests.rs b/runtime/src/serde_snapshot/tests.rs index 898a84673f..862a3dbbfc 100644 --- a/runtime/src/serde_snapshot/tests.rs +++ b/runtime/src/serde_snapshot/tests.rs @@ -367,7 +367,7 @@ mod test_bank_serialize { // This some what long test harness is required to freeze the ABI of // Bank's serialization due to versioned nature - #[frozen_abi(digest = "H2XtVdhokwLMTbjXh4Lh3Mw8m7PYQDMh4Ha5ojuxip9Z")] + #[frozen_abi(digest = "DnT7iwf29YvGxLTt2iLYPmixwP13LXvusVVKpXGgizhc")] #[derive(Serialize, AbiExample)] pub struct BankAbiTestWrapperNewer { #[serde(serialize_with = "wrapper_newer")]