From 561808cf90ebbb825c86c7f140ccd06f1f1d2226 Mon Sep 17 00:00:00 2001 From: Trent Nelson Date: Wed, 4 Mar 2020 12:01:32 -0700 Subject: [PATCH] SDK: Store FeeCalculator in recent_blockhashes sysvar (#8609) * SDK: Store FeeCalculators in recent_blockhashes sysvar * nits --- runtime/src/bank.rs | 4 +- runtime/src/blockhash_queue.rs | 16 ++- runtime/src/nonce_utils.rs | 4 +- runtime/src/system_instruction_processor.rs | 17 ++- sdk/src/nonce/account.rs | 25 ++-- sdk/src/sysvar/recent_blockhashes.rs | 119 +++++++++++++++----- 6 files changed, 131 insertions(+), 54 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 0cdc5fd39e..455448ab38 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -4991,9 +4991,9 @@ mod tests { sysvar::recent_blockhashes::RecentBlockhashes::from_account(&bhq_account).unwrap(); // Check length assert_eq!(recent_blockhashes.len(), i); - let most_recent_hash = recent_blockhashes.iter().nth(0).unwrap(); + let most_recent_hash = recent_blockhashes.iter().nth(0).unwrap().blockhash; // Check order - assert_eq!(Some(true), bank.check_hash_age(most_recent_hash, 0)); + assert_eq!(Some(true), bank.check_hash_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 71fc6cce44..16020d1a83 100644 --- a/runtime/src/blockhash_queue.rs +++ b/runtime/src/blockhash_queue.rs @@ -1,5 +1,7 @@ use serde::{Deserialize, Serialize}; -use solana_sdk::{fee_calculator::FeeCalculator, hash::Hash, timing::timestamp}; +use solana_sdk::{ + fee_calculator::FeeCalculator, hash::Hash, sysvar::recent_blockhashes, timing::timestamp, +}; use std::collections::HashMap; #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] @@ -112,15 +114,19 @@ impl BlockhashQueue { None } - pub fn get_recent_blockhashes(&self) -> impl Iterator { - (&self.ages).iter().map(|(k, v)| (v.hash_height, k)) + pub fn get_recent_blockhashes(&self) -> impl Iterator { + (&self.ages) + .iter() + .map(|(k, v)| recent_blockhashes::IterItem(v.hash_height, k, &v.fee_calculator)) } } #[cfg(test)] mod tests { use super::*; use bincode::serialize; - use solana_sdk::{clock::MAX_RECENT_BLOCKHASHES, hash::hash}; + use solana_sdk::{ + clock::MAX_RECENT_BLOCKHASHES, hash::hash, sysvar::recent_blockhashes::IterItem, + }; #[test] fn test_register_hash() { @@ -172,7 +178,7 @@ mod tests { } let recent_blockhashes = blockhash_queue.get_recent_blockhashes(); // Verify that the returned hashes are most recent - for (_slot, hash) in recent_blockhashes { + for IterItem(_slot, hash, _fee_calc) in recent_blockhashes { assert_eq!( Some(true), blockhash_queue.check_hash_age(hash, MAX_RECENT_BLOCKHASHES) diff --git a/runtime/src/nonce_utils.rs b/runtime/src/nonce_utils.rs index 103aaadf6e..7218c3fd46 100644 --- a/runtime/src/nonce_utils.rs +++ b/runtime/src/nonce_utils.rs @@ -208,7 +208,7 @@ mod tests { .unwrap(); assert!(verify_nonce_account( &nonce_account.account.borrow(), - &recent_blockhashes[0] + &recent_blockhashes[0].blockhash, )); }); } @@ -238,7 +238,7 @@ mod tests { .unwrap(); assert!(!verify_nonce_account( &nonce_account.account.borrow(), - &recent_blockhashes[1] + &recent_blockhashes[1].blockhash, )); }); } diff --git a/runtime/src/system_instruction_processor.rs b/runtime/src/system_instruction_processor.rs index aef903c70b..7c7df51e90 100644 --- a/runtime/src/system_instruction_processor.rs +++ b/runtime/src/system_instruction_processor.rs @@ -324,6 +324,7 @@ mod tests { use solana_sdk::{ account::Account, client::SyncClient, + fee_calculator::FeeCalculator, genesis_config::create_genesis_config, hash::{hash, Hash}, instruction::{AccountMeta, Instruction, InstructionError}, @@ -331,6 +332,7 @@ mod tests { nonce, signature::{Keypair, Signer}, system_instruction, system_program, sysvar, + sysvar::recent_blockhashes::IterItem, transaction::TransactionError, }; use std::cell::RefCell; @@ -350,7 +352,7 @@ mod tests { fn create_default_recent_blockhashes_account() -> RefCell { RefCell::new(sysvar::recent_blockhashes::create_account_with_data( 1, - vec![(0u64, &Hash::default()); 32].into_iter(), + vec![IterItem(0u64, &Hash::default(), &FeeCalculator::default()); 32].into_iter(), )) } fn create_default_rent_account() -> RefCell { @@ -1011,7 +1013,8 @@ mod tests { RefCell::new(if sysvar::recent_blockhashes::check_id(&meta.pubkey) { sysvar::recent_blockhashes::create_account_with_data( 1, - vec![(0u64, &Hash::default()); 32].into_iter(), + vec![IterItem(0u64, &Hash::default(), &FeeCalculator::default()); 32] + .into_iter(), ) } else if sysvar::rent::check_id(&meta.pubkey) { sysvar::rent::create_account(1, &Rent::free()) @@ -1110,7 +1113,15 @@ mod tests { let new_recent_blockhashes_account = RefCell::new(sysvar::recent_blockhashes::create_account_with_data( 1, - vec![(0u64, &hash(&serialize(&0).unwrap())); 32].into_iter(), + vec![ + IterItem( + 0u64, + &hash(&serialize(&0).unwrap()), + &FeeCalculator::default() + ); + 32 + ] + .into_iter(), )); assert_eq!( super::process_instruction( diff --git a/sdk/src/nonce/account.rs b/sdk/src/nonce/account.rs index 62a89be4f6..1ecbc5a03f 100644 --- a/sdk/src/nonce/account.rs +++ b/sdk/src/nonce/account.rs @@ -53,12 +53,13 @@ impl<'a> Account for KeyedAccount<'a> { if !signers.contains(&data.authority) { return Err(InstructionError::MissingRequiredSignature); } - if data.blockhash == recent_blockhashes[0] { + let recent_blockhash = recent_blockhashes[0].blockhash; + if data.blockhash == recent_blockhash { return Err(NonceError::NotExpired.into()); } let new_data = nonce::state::Data { - blockhash: recent_blockhashes[0], + blockhash: recent_blockhash, ..data }; self.set_state(&Versions::new_current(State::Initialized(new_data))) @@ -84,7 +85,7 @@ impl<'a> Account for KeyedAccount<'a> { } State::Initialized(ref data) => { if lamports == self.lamports()? { - if data.blockhash == recent_blockhashes[0] { + if data.blockhash == recent_blockhashes[0].blockhash { return Err(NonceError::NotExpired.into()); } } else { @@ -125,7 +126,7 @@ impl<'a> Account for KeyedAccount<'a> { } let data = nonce::state::Data { authority: *nonce_authority, - blockhash: recent_blockhashes[0], + blockhash: recent_blockhashes[0].blockhash, }; self.set_state(&Versions::new_current(State::Initialized(data))) } @@ -223,7 +224,7 @@ mod test { .unwrap() .convert_to_current(); let data = nonce::state::Data { - blockhash: recent_blockhashes[0], + blockhash: recent_blockhashes[0].blockhash, ..data }; // First nonce instruction drives state from Uninitialized to Initialized @@ -236,7 +237,7 @@ mod test { .unwrap() .convert_to_current(); let data = nonce::state::Data { - blockhash: recent_blockhashes[0], + blockhash: recent_blockhashes[0].blockhash, ..data }; // Second nonce instruction consumes and replaces stored nonce @@ -249,7 +250,7 @@ mod test { .unwrap() .convert_to_current(); let data = nonce::state::Data { - blockhash: recent_blockhashes[0], + blockhash: recent_blockhashes[0].blockhash, ..data }; // Third nonce instruction for fun and profit @@ -300,7 +301,7 @@ mod test { .convert_to_current(); let data = nonce::state::Data { authority, - blockhash: recent_blockhashes[0], + blockhash: recent_blockhashes[0].blockhash, }; assert_eq!(state, State::Initialized(data)); let signers = HashSet::new(); @@ -588,7 +589,7 @@ mod test { .convert_to_current(); let data = nonce::state::Data { authority, - blockhash: recent_blockhashes[0], + blockhash: recent_blockhashes[0].blockhash, }; assert_eq!(state, State::Initialized(data)); with_test_keyed_account(42, false, |to_keyed| { @@ -609,7 +610,7 @@ mod test { .unwrap() .convert_to_current(); let data = nonce::state::Data { - blockhash: recent_blockhashes[0], + blockhash: recent_blockhashes[0].blockhash, ..data }; assert_eq!(state, State::Initialized(data)); @@ -744,7 +745,7 @@ mod test { keyed_account.initialize_nonce_account(&authority, &recent_blockhashes, &rent); let data = nonce::state::Data { authority, - blockhash: recent_blockhashes[0], + blockhash: recent_blockhashes[0].blockhash, }; assert_eq!(result, Ok(())); let state = AccountUtilsState::::state(keyed_account) @@ -826,7 +827,7 @@ mod test { let authority = Pubkey::default(); let data = nonce::state::Data { authority, - blockhash: recent_blockhashes[0], + blockhash: recent_blockhashes[0].blockhash, }; let result = nonce_account.authorize_nonce_account(&Pubkey::default(), &signers); assert_eq!(result, Ok(())); diff --git a/sdk/src/sysvar/recent_blockhashes.rs b/sdk/src/sysvar/recent_blockhashes.rs index 9d30674a14..d72ae08598 100644 --- a/sdk/src/sysvar/recent_blockhashes.rs +++ b/sdk/src/sysvar/recent_blockhashes.rs @@ -1,21 +1,61 @@ use crate::{ account::Account, + declare_sysvar_id, + fee_calculator::FeeCalculator, hash::{hash, Hash}, sysvar::Sysvar, }; -use bincode::serialize; -use std::{collections::BinaryHeap, iter::FromIterator, ops::Deref}; +use std::{cmp::Ordering, collections::BinaryHeap, iter::FromIterator, ops::Deref}; const MAX_ENTRIES: usize = 32; -crate::declare_sysvar_id!( +declare_sysvar_id!( "SysvarRecentB1ockHashes11111111111111111111", RecentBlockhashes ); #[repr(C)] -#[derive(Serialize, Deserialize, Debug, PartialEq)] -pub struct RecentBlockhashes(Vec); +#[derive(Serialize, Deserialize, Clone, Debug, Default, PartialEq)] +pub struct Entry { + pub blockhash: Hash, + pub fee_calculator: FeeCalculator, +} + +impl Entry { + pub fn new(blockhash: &Hash, fee_calculator: &FeeCalculator) -> Self { + Self { + blockhash: *blockhash, + fee_calculator: fee_calculator.clone(), + } + } +} + +#[derive(Clone, Debug)] +pub struct IterItem<'a>(pub u64, pub &'a Hash, pub &'a FeeCalculator); + +impl<'a> Eq for IterItem<'a> {} + +impl<'a> PartialEq for IterItem<'a> { + fn eq(&self, other: &Self) -> bool { + self.0 == other.0 + } +} + +impl<'a> Ord for IterItem<'a> { + fn cmp(&self, other: &Self) -> Ordering { + self.0.cmp(&other.0) + } +} + +impl<'a> PartialOrd for IterItem<'a> { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +#[repr(C)] +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq)] +pub struct RecentBlockhashes(Vec); impl Default for RecentBlockhashes { fn default() -> Self { @@ -23,14 +63,14 @@ impl Default for RecentBlockhashes { } } -impl<'a> FromIterator<&'a Hash> for RecentBlockhashes { +impl<'a> FromIterator> for RecentBlockhashes { fn from_iter(iter: I) -> Self where - I: IntoIterator, + I: IntoIterator>, { let mut new = Self::default(); for i in iter { - new.0.push(*i) + new.0.push(Entry::new(i.1, i.2)) } new } @@ -67,12 +107,12 @@ impl Iterator for IntoIterSorted { impl Sysvar for RecentBlockhashes { fn size_of() -> usize { // hard-coded so that we don't have to construct an empty - 1032 // golden, update if MAX_ENTRIES changes + 1288 // golden, update if MAX_ENTRIES changes } } impl Deref for RecentBlockhashes { - type Target = Vec; + type Target = Vec; fn deref(&self) -> &Self::Target { &self.0 } @@ -84,18 +124,18 @@ pub fn create_account(lamports: u64) -> Account { pub fn update_account<'a, I>(account: &mut Account, recent_blockhash_iter: I) -> Option<()> where - I: IntoIterator, + I: IntoIterator>, { let sorted = BinaryHeap::from_iter(recent_blockhash_iter); let sorted_iter = IntoIterSorted { inner: sorted }; - let recent_blockhash_iter = sorted_iter.take(MAX_ENTRIES).map(|(_, hash)| hash); + let recent_blockhash_iter = sorted_iter.take(MAX_ENTRIES); let recent_blockhashes = RecentBlockhashes::from_iter(recent_blockhash_iter); recent_blockhashes.to_account(account) } pub fn create_account_with_data<'a, I>(lamports: u64, recent_blockhash_iter: I) -> Account where - I: IntoIterator, + I: IntoIterator>, { let mut account = create_account(lamports); update_account(&mut account, recent_blockhash_iter).unwrap(); @@ -103,10 +143,15 @@ where } pub fn create_test_recent_blockhashes(start: usize) -> RecentBlockhashes { - let bhq: Vec<_> = (start..start + MAX_ENTRIES) - .map(|i| hash(&serialize(&i).unwrap())) + let blocks: Vec<_> = (start..start + MAX_ENTRIES) + .map(|i| (i as u64, hash(&bincode::serialize(&i).unwrap()))) .collect(); - RecentBlockhashes::from_iter(bhq.iter()) + let def_fees = FeeCalculator::default(); + let bhq: Vec<_> = blocks + .iter() + .map(|(i, hash)| IterItem(*i, hash, &def_fees)) + .collect(); + RecentBlockhashes::from_iter(bhq.into_iter()) } #[cfg(test)] @@ -118,9 +163,10 @@ mod tests { #[test] fn test_size_of() { + let entry = Entry::new(&Hash::default(), &FeeCalculator::default()); assert_eq!( - bincode::serialized_size(&RecentBlockhashes(vec![Hash::default(); MAX_ENTRIES])) - .unwrap() as usize, + bincode::serialized_size(&RecentBlockhashes(vec![entry; MAX_ENTRIES])).unwrap() + as usize, RecentBlockhashes::size_of() ); } @@ -135,8 +181,11 @@ mod tests { #[test] fn test_create_account_full() { let def_hash = Hash::default(); - let account = - create_account_with_data(42, vec![(0u64, &def_hash); MAX_ENTRIES].into_iter()); + let def_fees = FeeCalculator::default(); + let account = create_account_with_data( + 42, + vec![IterItem(0u64, &def_hash, &def_fees); MAX_ENTRIES].into_iter(), + ); let recent_blockhashes = RecentBlockhashes::from_account(&account).unwrap(); assert_eq!(recent_blockhashes.len(), MAX_ENTRIES); } @@ -144,15 +193,19 @@ mod tests { #[test] fn test_create_account_truncate() { let def_hash = Hash::default(); - let account = - create_account_with_data(42, vec![(0u64, &def_hash); MAX_ENTRIES + 1].into_iter()); + let def_fees = FeeCalculator::default(); + let account = create_account_with_data( + 42, + vec![IterItem(0u64, &def_hash, &def_fees); MAX_ENTRIES + 1].into_iter(), + ); let recent_blockhashes = RecentBlockhashes::from_account(&account).unwrap(); assert_eq!(recent_blockhashes.len(), MAX_ENTRIES); } #[test] fn test_create_account_unsorted() { - let mut unsorted_recent_blockhashes: Vec<_> = (0..MAX_ENTRIES) + let def_fees = FeeCalculator::default(); + let mut unsorted_blocks: Vec<_> = (0..MAX_ENTRIES) .map(|i| { (i as u64, { // create hash with visibly recognizable ordering @@ -162,20 +215,26 @@ mod tests { }) }) .collect(); - unsorted_recent_blockhashes.shuffle(&mut thread_rng()); + unsorted_blocks.shuffle(&mut thread_rng()); let account = create_account_with_data( 42, - unsorted_recent_blockhashes + unsorted_blocks .iter() - .map(|(i, hash)| (*i, hash)), + .map(|(i, hash)| IterItem(*i, hash, &def_fees)), ); let recent_blockhashes = RecentBlockhashes::from_account(&account).unwrap(); - let mut expected_recent_blockhashes: Vec<_> = - (unsorted_recent_blockhashes.into_iter().map(|(_, b)| b)).collect(); - expected_recent_blockhashes.sort(); - expected_recent_blockhashes.reverse(); + let mut unsorted_recent_blockhashes: Vec<_> = unsorted_blocks + .iter() + .map(|(i, hash)| IterItem(*i, hash, &def_fees)) + .collect(); + unsorted_recent_blockhashes.sort(); + unsorted_recent_blockhashes.reverse(); + let expected_recent_blockhashes: Vec<_> = (unsorted_recent_blockhashes + .into_iter() + .map(|IterItem(_, b, f)| Entry::new(b, f))) + .collect(); assert_eq!(*recent_blockhashes, expected_recent_blockhashes); }