Skip to content

Commit

Permalink
Manage durable nonce stored value in runtime (#7684)
Browse files Browse the repository at this point in the history 
* Bank: Return nonce pubkey/account from `check_tx_durable_nonce`

* Forward account with HashAgeKind::DurableNonce

* Add durable nonce helper for HashAgeKind

* Add nonce util for advancing stored nonce in runtime

* Advance nonce in runtime

* Store rolled back nonce account on TX InstructionError

* nonce: Add test for replayed InstErr fee theft
  • Loading branch information
@t-nelson
t-nelson authored Jan 10, 2020
1 parent fd3c6eb commit 9754fc7
Show file tree
Hide file tree
Showing 4 changed files with 269 additions and 33 deletions.
2 changes: 1 addition & 1 deletion core/src/transaction_status_service.rs
View file
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
View file
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
View file
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

0 comments on commit 9754fc7

Please sign in to comment.