Store FeeCalculator with blockhash in nonce accounts (#8650)

* Copy current state version to v0

* Add `FeeCalculator` to nonce state

* fixup compile

* Dump v0 handling...

Since we new account data is all zeros, new `Current` versioned accounts
look like v0. We could hack around this with some data size checks, but
the `account_utils::*State` traits are applied to `Account`, not the
state data, so we're kind SOL...

* Create more representative test `RecentBlockhashes`

* Improve CLI nonce account display

Co-Authored-By: Michael Vines <mvines@gmail.com>

* Fix that last bank test...

* clippy/fmt

Co-authored-by: Michael Vines <mvines@gmail.com>
This commit is contained in:
Trent Nelson 2020-03-05 07:40:26 -07:00 committed by GitHub
parent 44fde2d964
commit fd00e5cb35
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 133 additions and 81 deletions

View File

@ -3269,6 +3269,7 @@ mod tests {
nonce::state::Versions::new_current(nonce::State::Initialized(nonce::state::Data {
authority: config.signers[0].pubkey(),
blockhash,
fee_calculator: FeeCalculator::default(),
}));
let nonce_response = json!(Response {
context: RpcResponseContext { slot: 1 },
@ -3297,6 +3298,7 @@ mod tests {
nonce::state::Versions::new_current(nonce::State::Initialized(nonce::state::Data {
authority: bob_pubkey,
blockhash,
fee_calculator: FeeCalculator::default(),
}));
let nonce_authority_response = json!(Response {
context: RpcResponseContext { slot: 1 },

View File

@ -569,10 +569,15 @@ pub fn process_show_nonce_account(
match data {
Some(ref data) => {
println!("Nonce: {}", data.blockhash);
println!(
"Fee: {} lamports per signature",
data.fee_calculator.lamports_per_signature
);
println!("Authority: {}", data.authority);
}
None => {
println!("Nonce: uninitialized");
println!("Fees: uninitialized");
println!("Authority: uninitialized");
}
}
@ -626,6 +631,7 @@ mod tests {
use crate::cli::{app, parse_command};
use solana_sdk::{
account::Account,
fee_calculator::FeeCalculator,
hash::hash,
nonce::{self, State},
signature::{read_keypair_file, write_keypair, Keypair, Signer},
@ -906,6 +912,7 @@ mod tests {
let data = Versions::new_current(State::Initialized(nonce::state::Data {
authority: nonce_pubkey,
blockhash,
fee_calculator: FeeCalculator::default(),
}));
let valid = Account::new_data(1, &data, &system_program::ID);
assert!(check_nonce_account(&valid.unwrap(), &nonce_pubkey, &blockhash).is_ok());
@ -929,6 +936,7 @@ mod tests {
let data = Versions::new_current(State::Initialized(nonce::state::Data {
authority: nonce_pubkey,
blockhash: hash(b"invalid"),
fee_calculator: FeeCalculator::default(),
}));
let invalid_hash = Account::new_data(1, &data, &system_program::ID);
assert_eq!(
@ -939,6 +947,7 @@ mod tests {
let data = Versions::new_current(State::Initialized(nonce::state::Data {
authority: Pubkey::new_rand(),
blockhash,
fee_calculator: FeeCalculator::default(),
}));
let invalid_authority = Account::new_data(1, &data, &system_program::ID);
assert_eq!(

View File

@ -1,7 +1,10 @@
use crossbeam_channel::{Receiver, RecvTimeoutError};
use solana_client::rpc_response::RpcTransactionStatus;
use solana_ledger::{blockstore::Blockstore, blockstore_processor::TransactionStatusBatch};
use solana_runtime::bank::{Bank, HashAgeKind};
use solana_runtime::{
bank::{Bank, HashAgeKind},
nonce_utils,
};
use std::{
sync::{
atomic::{AtomicBool, Ordering},
@ -59,14 +62,13 @@ impl TransactionStatusService {
.zip(balances.post_balances)
{
if Bank::can_commit(&status) && !transaction.signatures.is_empty() {
let fee_hash = if let Some(HashAgeKind::DurableNonce(_, _)) = hash_age_kind {
bank.last_blockhash()
} else {
transaction.message().recent_blockhash
};
let fee_calculator = bank
.get_fee_calculator(&fee_hash)
.expect("FeeCalculator must exist");
let fee_calculator = match hash_age_kind {
Some(HashAgeKind::DurableNonce(_, account)) => {
nonce_utils::fee_calculator_of(&account)
}
_ => bank.get_fee_calculator(&transaction.message().recent_blockhash),
}
.expect("FeeCalculator must exist");
let fee = fee_calculator.calculate_fee(transaction.message());
blockstore
.write_transaction_status(

View File

@ -6,7 +6,7 @@ use crate::{
append_vec::StoredAccount,
bank::{HashAgeKind, TransactionProcessResult},
blockhash_queue::BlockhashQueue,
nonce_utils::prepare_if_nonce_account,
nonce_utils,
rent_collector::RentCollector,
system_instruction_processor::{get_system_account_kind, SystemAccountKind},
transaction_utils::OrderedIterator,
@ -265,13 +265,15 @@ impl Accounts {
.zip(lock_results.into_iter())
.map(|etx| match etx {
(tx, (Ok(()), hash_age_kind)) => {
let fee_hash = if let Some(HashAgeKind::DurableNonce(_, _)) = hash_age_kind {
hash_queue.last_hash()
} else {
tx.message().recent_blockhash
let fee_calculator = match hash_age_kind.as_ref() {
Some(HashAgeKind::DurableNonce(_, account)) => {
nonce_utils::fee_calculator_of(account)
}
_ => hash_queue
.get_fee_calculator(&tx.message().recent_blockhash)
.cloned(),
};
let fee = if let Some(fee_calculator) = hash_queue.get_fee_calculator(&fee_hash)
{
let fee = if let Some(fee_calculator) = fee_calculator {
fee_calculator.calculate_fee(tx.message())
} else {
return (Err(TransactionError::BlockhashNotFound), hash_age_kind);
@ -630,7 +632,13 @@ impl Accounts {
.enumerate()
.zip(acc.0.iter_mut())
{
prepare_if_nonce_account(account, key, res, maybe_nonce, last_blockhash);
nonce_utils::prepare_if_nonce_account(
account,
key,
res,
maybe_nonce,
last_blockhash,
);
if message.is_writable(i) {
if account.rent_epoch == 0 {
account.rent_epoch = rent_collector.epoch;
@ -921,10 +929,9 @@ mod tests {
nonce.pubkey(),
Account::new_data(
min_balance * 2,
&nonce::State::Initialized(nonce::state::Data {
authority: Pubkey::default(),
blockhash: Hash::default(),
}),
&nonce::state::Versions::new_current(nonce::State::Initialized(
nonce::state::Data::default(),
)),
&system_program::id(),
)
.unwrap(),

View File

@ -1409,19 +1409,20 @@ impl Bank {
let results = OrderedIterator::new(txs, iteration_order)
.zip(executed.iter())
.map(|(tx, (res, hash_age_kind))| {
let is_durable_nonce = hash_age_kind
.as_ref()
.map(|hash_age_kind| hash_age_kind.is_durable_nonce())
.unwrap_or(false);
let fee_hash = if is_durable_nonce {
self.last_blockhash()
} else {
tx.message().recent_blockhash
let (fee_calculator, is_durable_nonce) = match hash_age_kind {
Some(HashAgeKind::DurableNonce(_, account)) => {
(nonce_utils::fee_calculator_of(account), true)
}
_ => (
hash_queue
.get_fee_calculator(&tx.message().recent_blockhash)
.cloned(),
false,
),
};
let fee = hash_queue
.get_fee_calculator(&fee_hash)
.ok_or(TransactionError::BlockhashNotFound)?
.calculate_fee(tx.message());
let fee_calculator = fee_calculator.ok_or(TransactionError::BlockhashNotFound)?;
let fee = fee_calculator.calculate_fee(tx.message());
let message = tx.message();
match *res {
@ -3417,10 +3418,9 @@ mod tests {
let nonce = Keypair::new();
let nonce_account = Account::new_data(
min_balance + 42,
&nonce::State::Initialized(nonce::state::Data {
authority: Pubkey::default(),
blockhash: Hash::default(),
}),
&nonce::state::Versions::new_current(nonce::State::Initialized(
nonce::state::Data::default(),
)),
&system_program::id(),
)
.unwrap();
@ -5153,6 +5153,13 @@ mod tests {
genesis_cfg_fn(&mut genesis_config);
let mut bank = Arc::new(Bank::new(&genesis_config));
// Banks 0 and 1 have no fees, wait two blocks before
// initializing our nonce accounts
for _ in 0..2 {
goto_end_of_slot(Arc::get_mut(&mut bank).unwrap());
bank = Arc::new(new_from_parent(&bank));
}
let (custodian_keypair, nonce_keypair) = nonce_setup(
&mut bank,
&mint_keypair,
@ -5276,10 +5283,9 @@ mod tests {
let nonce = Keypair::new();
let nonce_account = Account::new_data(
42424242,
&nonce::State::Initialized(nonce::state::Data {
authority: Pubkey::default(),
blockhash: Hash::default(),
}),
&nonce::state::Versions::new_current(nonce::State::Initialized(
nonce::state::Data::default(),
)),
&system_program::id(),
)
.unwrap();

View File

@ -10,7 +10,7 @@ pub mod genesis_utils;
pub mod loader_utils;
pub mod message_processor;
mod native_loader;
mod nonce_utils;
pub mod nonce_utils;
pub mod rent_collector;
mod serde_utils;
pub mod stakes;

View File

@ -1,6 +1,7 @@
use solana_sdk::{
account::Account,
account_utils::StateMut,
fee_calculator::FeeCalculator,
hash::Hash,
instruction::CompiledInstruction,
nonce::{self, state::Versions, State},
@ -66,7 +67,7 @@ pub fn prepare_if_nonce_account(
if let State::Initialized(ref data) = state {
let new_data = Versions::new_current(State::Initialized(nonce::state::Data {
blockhash: *last_blockhash,
..*data
..data.clone()
}));
account.set_state(&new_data).unwrap();
}
@ -74,6 +75,16 @@ pub fn prepare_if_nonce_account(
}
}
pub fn fee_calculator_of(account: &Account) -> Option<FeeCalculator> {
let state = StateMut::<Versions>::state(account)
.ok()?
.convert_to_current();
match state {
State::Initialized(data) => Some(data.fee_calculator),
_ => None,
}
}
#[cfg(test)]
mod tests {
use super::*;
@ -244,10 +255,7 @@ mod tests {
}
fn create_accounts_prepare_if_nonce_account() -> (Pubkey, Account, Account, Hash) {
let data = Versions::new_current(State::Initialized(nonce::state::Data {
authority: Pubkey::default(),
blockhash: Hash::default(),
}));
let data = Versions::new_current(State::Initialized(nonce::state::Data::default()));
let account = Account::new_data(42, &data, &system_program::id()).unwrap();
let pre_account = Account {
lamports: 43,
@ -300,8 +308,8 @@ mod tests {
let mut expect_account = post_account.clone();
let data = Versions::new_current(State::Initialized(nonce::state::Data {
authority: Pubkey::default(),
blockhash: last_blockhash,
..nonce::state::Data::default()
}));
expect_account.set_state(&data).unwrap();
@ -360,8 +368,8 @@ mod tests {
expect_account
.set_state(&Versions::new_current(State::Initialized(
nonce::state::Data {
authority: Pubkey::default(),
blockhash: last_blockhash,
..nonce::state::Data::default()
},
)))
.unwrap();

View File

@ -306,8 +306,13 @@ pub fn get_system_account_kind(account: &Account) -> Option<SystemAccountKind> {
if system_program::check_id(&account.owner) {
if account.data.is_empty() {
Some(SystemAccountKind::System)
} else if let Ok(nonce::State::Initialized(_)) = account.state() {
Some(SystemAccountKind::Nonce)
} else if account.data.len() == nonce::State::size() {
match account.state().ok()? {
nonce::state::Versions::Current(state) => match *state {
nonce::State::Initialized(_) => Some(SystemAccountKind::Nonce),
_ => None,
},
}
} else {
None
}
@ -735,10 +740,9 @@ mod tests {
let nonce = Pubkey::new_rand();
let nonce_account = Account::new_ref_data(
42,
&nonce::State::Initialized(nonce::state::Data {
authority: Pubkey::default(),
blockhash: Hash::default(),
}),
&nonce::state::Versions::new_current(nonce::State::Initialized(
nonce::state::Data::default(),
)),
&system_program::id(),
)
.unwrap();
@ -877,10 +881,10 @@ mod tests {
let from = Pubkey::new_rand();
let from_account = Account::new_ref_data(
100,
&nonce::State::Initialized(nonce::state::Data {
&nonce::state::Versions::new_current(nonce::State::Initialized(nonce::state::Data {
authority: from,
blockhash: Hash::default(),
}),
..nonce::state::Data::default()
})),
&system_program::id(),
)
.unwrap();
@ -1382,10 +1386,9 @@ mod tests {
fn test_get_system_account_kind_nonce_ok() {
let nonce_account = Account::new_data(
42,
&nonce::State::Initialized(nonce::state::Data {
authority: Pubkey::default(),
blockhash: Hash::default(),
}),
&nonce::state::Versions::new_current(nonce::State::Initialized(
nonce::state::Data::default(),
)),
&system_program::id(),
)
.unwrap();
@ -1413,10 +1416,9 @@ mod tests {
fn test_get_system_account_kind_nonsystem_owner_with_nonce_data_fail() {
let nonce_account = Account::new_data(
42,
&nonce::State::Initialized(nonce::state::Data {
authority: Pubkey::default(),
blockhash: Hash::default(),
}),
&nonce::state::Versions::new_current(nonce::State::Initialized(
nonce::state::Data::default(),
)),
&Pubkey::new_rand(),
)
.unwrap();

View File

@ -60,6 +60,7 @@ impl<'a> Account for KeyedAccount<'a> {
let new_data = nonce::state::Data {
blockhash: recent_blockhash,
fee_calculator: recent_blockhashes[0].fee_calculator.clone(),
..data
};
self.set_state(&Versions::new_current(State::Initialized(new_data)))
@ -127,6 +128,7 @@ impl<'a> Account for KeyedAccount<'a> {
let data = nonce::state::Data {
authority: *nonce_authority,
blockhash: recent_blockhashes[0].blockhash,
fee_calculator: recent_blockhashes[0].fee_calculator.clone(),
};
self.set_state(&Versions::new_current(State::Initialized(data)))
}
@ -159,7 +161,7 @@ pub fn create_account(lamports: u64) -> RefCell<account::Account> {
RefCell::new(
account::Account::new_data_with_space(
lamports,
&State::Uninitialized,
&Versions::new_current(State::Uninitialized),
State::size(),
&system_program::id(),
)
@ -225,10 +227,11 @@ mod test {
.convert_to_current();
let data = nonce::state::Data {
blockhash: recent_blockhashes[0].blockhash,
..data
fee_calculator: recent_blockhashes[0].fee_calculator.clone(),
..data.clone()
};
// First nonce instruction drives state from Uninitialized to Initialized
assert_eq!(state, State::Initialized(data));
assert_eq!(state, State::Initialized(data.clone()));
let recent_blockhashes = create_test_recent_blockhashes(63);
keyed_account
.advance_nonce_account(&recent_blockhashes, &signers)
@ -238,10 +241,11 @@ mod test {
.convert_to_current();
let data = nonce::state::Data {
blockhash: recent_blockhashes[0].blockhash,
..data
fee_calculator: recent_blockhashes[0].fee_calculator.clone(),
..data.clone()
};
// Second nonce instruction consumes and replaces stored nonce
assert_eq!(state, State::Initialized(data));
assert_eq!(state, State::Initialized(data.clone()));
let recent_blockhashes = create_test_recent_blockhashes(31);
keyed_account
.advance_nonce_account(&recent_blockhashes, &signers)
@ -251,10 +255,11 @@ mod test {
.convert_to_current();
let data = nonce::state::Data {
blockhash: recent_blockhashes[0].blockhash,
..data
fee_calculator: recent_blockhashes[0].fee_calculator.clone(),
..data.clone()
};
// Third nonce instruction for fun and profit
assert_eq!(state, State::Initialized(data));
assert_eq!(state, State::Initialized(data.clone()));
with_test_keyed_account(42, false, |to_keyed| {
let recent_blockhashes = create_test_recent_blockhashes(0);
let withdraw_lamports = keyed_account.account.borrow().lamports;
@ -302,6 +307,7 @@ mod test {
let data = nonce::state::Data {
authority,
blockhash: recent_blockhashes[0].blockhash,
fee_calculator: recent_blockhashes[0].fee_calculator.clone(),
};
assert_eq!(state, State::Initialized(data));
let signers = HashSet::new();
@ -590,8 +596,9 @@ mod test {
let data = nonce::state::Data {
authority,
blockhash: recent_blockhashes[0].blockhash,
fee_calculator: recent_blockhashes[0].fee_calculator.clone(),
};
assert_eq!(state, State::Initialized(data));
assert_eq!(state, State::Initialized(data.clone()));
with_test_keyed_account(42, false, |to_keyed| {
let withdraw_lamports = nonce_keyed.account.borrow().lamports - min_lamports;
let nonce_expect_lamports =
@ -611,7 +618,8 @@ mod test {
.convert_to_current();
let data = nonce::state::Data {
blockhash: recent_blockhashes[0].blockhash,
..data
fee_calculator: recent_blockhashes[0].fee_calculator.clone(),
..data.clone()
};
assert_eq!(state, State::Initialized(data));
assert_eq!(nonce_keyed.account.borrow().lamports, nonce_expect_lamports);
@ -746,6 +754,7 @@ mod test {
let data = nonce::state::Data {
authority,
blockhash: recent_blockhashes[0].blockhash,
fee_calculator: recent_blockhashes[0].fee_calculator.clone(),
};
assert_eq!(result, Ok(()));
let state = AccountUtilsState::<Versions>::state(keyed_account)
@ -828,6 +837,7 @@ mod test {
let data = nonce::state::Data {
authority,
blockhash: recent_blockhashes[0].blockhash,
fee_calculator: recent_blockhashes[0].fee_calculator.clone(),
};
let result = nonce_account.authorize_nonce_account(&Pubkey::default(), &signers);
assert_eq!(result, Ok(()));

View File

@ -1,14 +1,15 @@
use super::Versions;
use crate::{hash::Hash, pubkey::Pubkey};
use crate::{fee_calculator::FeeCalculator, hash::Hash, pubkey::Pubkey};
use serde_derive::{Deserialize, Serialize};
#[derive(Debug, Default, Serialize, Deserialize, PartialEq, Clone, Copy)]
#[derive(Debug, Default, Serialize, Deserialize, PartialEq, Clone)]
pub struct Data {
pub authority: Pubkey,
pub blockhash: Hash,
pub fee_calculator: FeeCalculator,
}
#[derive(Debug, Serialize, Deserialize, PartialEq, Clone, Copy)]
#[derive(Debug, Serialize, Deserialize, PartialEq, Clone)]
pub enum State {
Uninitialized,
Initialized(Data),

View File

@ -144,12 +144,17 @@ where
pub fn create_test_recent_blockhashes(start: usize) -> RecentBlockhashes {
let blocks: Vec<_> = (start..start + MAX_ENTRIES)
.map(|i| (i as u64, hash(&bincode::serialize(&i).unwrap())))
.map(|i| {
(
i as u64,
hash(&bincode::serialize(&i).unwrap()),
FeeCalculator::new(i as u64 * 100),
)
})
.collect();
let def_fees = FeeCalculator::default();
let bhq: Vec<_> = blocks
.iter()
.map(|(i, hash)| IterItem(*i, hash, &def_fees))
.map(|(i, hash, fee_calc)| IterItem(*i, hash, fee_calc))
.collect();
RecentBlockhashes::from_iter(bhq.into_iter())
}