Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Manage durable nonce stored value in runtime #7684

Merged
merged 7 commits into from
Jan 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/src/transaction_status_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ 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 {
let fee_hash = if let Some(HashAgeKind::DurableNonce(_, _)) = hash_age_kind {
bank.last_blockhash()
} else {
transaction.message().recent_blockhash
Expand Down
40 changes: 33 additions & 7 deletions runtime/src/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::append_vec::StoredAccount;
use crate::bank::{HashAgeKind, TransactionProcessResult};
use crate::blockhash_queue::BlockhashQueue;
use crate::message_processor::has_duplicates;
use crate::nonce_utils::prepare_if_nonce_account;
use crate::rent_collector::RentCollector;
use crate::system_instruction_processor::{get_system_account_kind, SystemAccountKind};
use log::*;
Expand All @@ -12,6 +13,7 @@ use solana_metrics::inc_new_counter_error;
use solana_sdk::account::Account;
use solana_sdk::bank_hash::BankHash;
use solana_sdk::clock::Slot;
use solana_sdk::hash::Hash;
use solana_sdk::native_loader;
use solana_sdk::nonce_state::NonceState;
use solana_sdk::pubkey::Pubkey;
Expand Down Expand Up @@ -247,7 +249,7 @@ 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 {
let fee_hash = if let Some(HashAgeKind::DurableNonce(_, _)) = hash_age_kind {
hash_queue.last_hash()
} else {
tx.message().recent_blockhash
Expand Down Expand Up @@ -559,9 +561,16 @@ impl Accounts {
res: &[TransactionProcessResult],
loaded: &mut [(Result<TransactionLoadResult>, Option<HashAgeKind>)],
rent_collector: &RentCollector,
last_blockhash: &Hash,
) {
let accounts_to_store =
self.collect_accounts_to_store(txs, txs_iteration_order, res, loaded, rent_collector);
let accounts_to_store = self.collect_accounts_to_store(
txs,
txs_iteration_order,
res,
loaded,
rent_collector,
last_blockhash,
);
self.accounts_db.store(slot, &accounts_to_store);
}

Expand All @@ -582,17 +591,27 @@ impl Accounts {
res: &'a [TransactionProcessResult],
loaded: &'a mut [(Result<TransactionLoadResult>, Option<HashAgeKind>)],
rent_collector: &RentCollector,
last_blockhash: &Hash,
) -> Vec<(&'a Pubkey, &'a Account)> {
let mut accounts = Vec::with_capacity(loaded.len());
for (i, ((raccs, _hash_age_kind), tx)) in loaded
.iter_mut()
.zip(OrderedIterator::new(txs, txs_iteration_order))
.enumerate()
{
let (res, _hash_age_kind) = &res[i];
if res.is_err() || raccs.is_err() {
if raccs.is_err() {
continue;
}
let (res, hash_age_kind) = &res[i];
let maybe_nonce = match (res, hash_age_kind) {
(Ok(_), Some(HashAgeKind::DurableNonce(pubkey, acc))) => Some((pubkey, acc)),
(
Err(TransactionError::InstructionError(_, _)),
Some(HashAgeKind::DurableNonce(pubkey, acc)),
) => Some((pubkey, acc)),
(Ok(_), _hash_age_kind) => None,
(Err(_), _hash_age_kind) => continue,
};

let message = &tx.message();
let acc = raccs.as_mut().unwrap();
Expand All @@ -602,6 +621,7 @@ impl Accounts {
.enumerate()
.zip(acc.0.iter_mut())
{
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;
Expand Down Expand Up @@ -1601,8 +1621,14 @@ mod tests {
},
);
}
let collected_accounts =
accounts.collect_accounts_to_store(&txs, None, &loaders, &mut loaded, &rent_collector);
let collected_accounts = accounts.collect_accounts_to_store(
&txs,
None,
&loaders,
&mut loaded,
&rent_collector,
&Hash::default(),
);
assert_eq!(collected_accounts.len(), 2);
assert!(collected_accounts
.iter()
Expand Down
82 changes: 61 additions & 21 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,16 @@ pub type TransactionBalances = Vec<Vec<u64>>;
#[derive(Clone, Debug, Eq, PartialEq)]
pub enum HashAgeKind {
Extant,
DurableNonce,
DurableNonce(Pubkey, Account),
}

impl HashAgeKind {
pub fn is_durable_nonce(&self) -> bool {
match self {
HashAgeKind::DurableNonce(_, _) => true,
_ => false,
}
}
}

/// Manager for the state of all accounts and programs after processing its entries.
Expand Down Expand Up @@ -982,8 +991,8 @@ impl Bank {
let message = tx.message();
if hash_queue.check_hash_age(&message.recent_blockhash, max_age) {
(Ok(()), Some(HashAgeKind::Extant))
} else if self.check_tx_durable_nonce(&tx) {
(Ok(()), Some(HashAgeKind::DurableNonce))
} else if let Some((pubkey, acc)) = self.check_tx_durable_nonce(&tx) {
(Ok(()), Some(HashAgeKind::DurableNonce(pubkey, acc)))
} else {
error_counters.reserve_blockhash += 1;
(Err(TransactionError::BlockhashNotFound), None)
Expand Down Expand Up @@ -1037,16 +1046,16 @@ impl Bank {
.check_hash_age(hash, max_age)
}

pub fn check_tx_durable_nonce(&self, tx: &Transaction) -> bool {
pub fn check_tx_durable_nonce(&self, tx: &Transaction) -> Option<(Pubkey, Account)> {
nonce_utils::transaction_uses_durable_nonce(&tx)
.and_then(|nonce_ix| nonce_utils::get_nonce_pubkey_from_instruction(&nonce_ix, &tx))
.and_then(|nonce_pubkey| self.get_account(&nonce_pubkey))
.map_or_else(
|| false,
|nonce_account| {
nonce_utils::verify_nonce(&nonce_account, &tx.message().recent_blockhash)
},
)
.and_then(|nonce_pubkey| {
self.get_account(&nonce_pubkey)
.map(|acc| (*nonce_pubkey, acc))
})
.filter(|(_pubkey, nonce_account)| {
nonce_utils::verify_nonce(nonce_account, &tx.message().recent_blockhash)
})
}

pub fn check_transactions(
Expand Down Expand Up @@ -1236,7 +1245,11 @@ impl Bank {
let results = OrderedIterator::new(txs, iteration_order)
.zip(executed.iter())
.map(|(tx, (res, hash_age_kind))| {
let fee_hash = if let Some(HashAgeKind::DurableNonce) = 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
Expand All @@ -1252,7 +1265,12 @@ impl Bank {
// credit the transaction fee even in case of InstructionError
// necessary to withdraw from account[0] here because previous
// work of doing so (in accounts.load()) is ignored by store_account()
self.withdraw(&message.account_keys[0], fee)?;
//
// ...except nonce accounts, which will have their post-load,
// pre-execute account state stored
if !is_durable_nonce {
self.withdraw(&message.account_keys[0], fee)?;
}
fees += fee;
Ok(())
}
Expand Down Expand Up @@ -1304,6 +1322,7 @@ impl Bank {
executed,
loaded_accounts,
&self.rent_collector,
&self.last_blockhash(),
);
self.collect_rent(executed, loaded_accounts);

Expand Down Expand Up @@ -1931,6 +1950,14 @@ mod tests {
use std::{io::Cursor, result, time::Duration};
use tempfile::TempDir;

#[test]
fn test_hash_age_kind_is_durable_nonce() {
assert!(
HashAgeKind::DurableNonce(Pubkey::default(), Account::default()).is_durable_nonce()
);
assert!(!HashAgeKind::Extant.is_durable_nonce());
}

#[test]
fn test_bank_unix_timestamp() {
let (genesis_config, _mint_keypair) = create_genesis_config(1);
Expand Down Expand Up @@ -4739,7 +4766,11 @@ mod tests {
&[&custodian_keypair, &nonce_keypair],
nonce_hash,
);
assert!(bank.check_tx_durable_nonce(&tx));
let nonce_account = bank.get_account(&nonce_pubkey).unwrap();
assert_eq!(
bank.check_tx_durable_nonce(&tx),
Some((nonce_pubkey, nonce_account))
);
}

#[test]
Expand All @@ -4759,7 +4790,7 @@ mod tests {
&[&custodian_keypair, &nonce_keypair],
nonce_hash,
);
assert!(!bank.check_tx_durable_nonce(&tx));
assert!(bank.check_tx_durable_nonce(&tx).is_none());
}

#[test]
Expand All @@ -4780,7 +4811,7 @@ mod tests {
nonce_hash,
);
tx.message.instructions[0].accounts.clear();
assert!(!bank.check_tx_durable_nonce(&tx));
assert!(bank.check_tx_durable_nonce(&tx).is_none());
}

#[test]
Expand All @@ -4802,7 +4833,7 @@ mod tests {
&[&custodian_keypair, &nonce_keypair],
nonce_hash,
);
assert!(!bank.check_tx_durable_nonce(&tx));
assert!(bank.check_tx_durable_nonce(&tx).is_none());
}

#[test]
Expand All @@ -4821,7 +4852,7 @@ mod tests {
&[&custodian_keypair, &nonce_keypair],
Hash::default(),
);
assert!(!bank.check_tx_durable_nonce(&tx));
assert!(bank.check_tx_durable_nonce(&tx).is_none());
}

#[test]
Expand Down Expand Up @@ -4934,10 +4965,11 @@ mod tests {
bank.process_transaction(&durable_tx),
Err(TransactionError::BlockhashNotFound)
);
/* Check fee not charged */
/* Check fee not charged and nonce not advanced */
assert_eq!(bank.get_balance(&custodian_pubkey), 4_640_000);
assert_eq!(new_nonce, get_nonce(&bank, &nonce_pubkey).unwrap());

let nonce_hash = get_nonce(&bank, &nonce_pubkey).unwrap();
let nonce_hash = new_nonce;

/* Kick nonce hash off the blockhash_queue */
for _ in 0..MAX_RECENT_BLOCKHASHES + 1 {
Expand All @@ -4961,8 +4993,16 @@ mod tests {
system_instruction::SystemError::ResultWithNegativeLamports.into()
))
);
/* Check fee charged */
/* Check fee charged and nonce has advanced */
assert_eq!(bank.get_balance(&custodian_pubkey), 4_630_000);
assert_ne!(nonce_hash, get_nonce(&bank, &nonce_pubkey).unwrap());
/* Confirm replaying a TX that failed with InstructionError::* now
* fails with TransactionError::BlockhashNotFound
*/
assert_eq!(
bank.process_transaction(&durable_tx),
Err(TransactionError::BlockhashNotFound),
);
}

#[test]
Expand Down
Loading