Skip to content

Commit

Permalink
Remove BlockhashQueue dependency from SVM related code (solana-labs#3…
Browse files Browse the repository at this point in the history
  • Loading branch information
pgarg66 authored Jan 26, 2024
1 parent e38848e commit 0d117d4
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 71 deletions.
2 changes: 1 addition & 1 deletion accounts-db/src/transaction_results.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use {
},
};

pub type TransactionCheckResult = (transaction::Result<()>, Option<NoncePartial>);
pub type TransactionCheckResult = (transaction::Result<()>, Option<NoncePartial>, Option<u64>);

pub struct TransactionResults {
pub fee_collection_results: Vec<transaction::Result<()>>,
Expand Down
30 changes: 16 additions & 14 deletions core/src/banking_stage/consumer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,9 @@ impl Consumer {
let pre_results = vec![Ok(()); txs.len()];
let check_results =
bank.check_transactions(txs, &pre_results, MAX_PROCESSING_AGE, &mut error_counters);
let check_results = check_results.into_iter().map(|(result, _nonce)| result);
let check_results = check_results
.into_iter()
.map(|(result, _nonce, _lamports)| result);
let mut output = self.process_and_record_transactions_with_pre_results(
bank,
txs,
Expand Down Expand Up @@ -787,7 +789,7 @@ impl Consumer {
valid_txs
.iter()
.enumerate()
.filter_map(|(index, (x, _h))| if x.is_ok() { Some(index) } else { None })
.filter_map(|(index, (x, _h, _lamports))| if x.is_ok() { Some(index) } else { None })
.collect_vec()
}
}
Expand Down Expand Up @@ -2488,24 +2490,24 @@ mod tests {
fn test_bank_filter_valid_transaction_indexes() {
assert_eq!(
Consumer::filter_valid_transaction_indexes(&[
(Err(TransactionError::BlockhashNotFound), None),
(Err(TransactionError::BlockhashNotFound), None),
(Ok(()), None),
(Err(TransactionError::BlockhashNotFound), None),
(Ok(()), None),
(Ok(()), None),
(Err(TransactionError::BlockhashNotFound), None, None),
(Err(TransactionError::BlockhashNotFound), None, None),
(Ok(()), None, None),
(Err(TransactionError::BlockhashNotFound), None, None),
(Ok(()), None, None),
(Ok(()), None, None),
]),
[2, 4, 5]
);

assert_eq!(
Consumer::filter_valid_transaction_indexes(&[
(Ok(()), None),
(Err(TransactionError::BlockhashNotFound), None),
(Err(TransactionError::BlockhashNotFound), None),
(Ok(()), None),
(Ok(()), None),
(Ok(()), None),
(Ok(()), None, None),
(Err(TransactionError::BlockhashNotFound), None, None),
(Err(TransactionError::BlockhashNotFound), None, None),
(Ok(()), None, None),
(Ok(()), None, None),
(Ok(()), None, None),
]),
[0, 3, 4, 5]
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ impl SchedulerController {
let fee_check_results: Vec<_> = check_results
.into_iter()
.zip(transactions)
.map(|((result, _nonce), tx)| {
.map(|((result, _nonce, _lamports), tx)| {
result?; // if there's already error do nothing
Consumer::check_fee_payer_unlocked(bank, tx.message(), &mut error_counters)
})
Expand Down Expand Up @@ -226,7 +226,7 @@ impl SchedulerController {
&mut error_counters,
);

for ((result, _nonce), id) in check_results.into_iter().zip(chunk.iter()) {
for ((result, _nonce, _lamports), id) in check_results.into_iter().zip(chunk.iter()) {
if result.is_err() {
saturating_add_assign!(self.count_metrics.num_dropped_on_age_and_status, 1);
self.container.remove_by_id(&id.id);
Expand Down
2 changes: 1 addition & 1 deletion core/src/banking_stage/unprocessed_transaction_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -776,7 +776,7 @@ impl ThreadLocalUnprocessedPackets {
.iter()
.enumerate()
.filter_map(
|(tx_index, (result, _))| if result.is_ok() { Some(tx_index) } else { None },
|(tx_index, (result, _, _))| if result.is_ok() { Some(tx_index) } else { None },
)
.collect_vec()
}
Expand Down
36 changes: 16 additions & 20 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4460,7 +4460,7 @@ impl Bank {
&hash_queue,
error_counters,
),
Err(e) => (Err(e.clone()), None),
Err(e) => (Err(e.clone()), None, None),
})
.collect()
}
Expand All @@ -4475,14 +4475,20 @@ impl Bank {
) -> TransactionCheckResult {
let recent_blockhash = tx.message().recent_blockhash();
if hash_queue.is_hash_valid_for_age(recent_blockhash, max_age) {
(Ok(()), None)
(
Ok(()),
None,
hash_queue.get_lamports_per_signature(tx.message().recent_blockhash()),
)
} else if let Some((address, account)) =
self.check_transaction_for_nonce(tx, next_durable_nonce)
{
(Ok(()), Some(NoncePartial::new(address, account)))
let nonce = NoncePartial::new(address, account);
let lamports_per_signature = nonce.lamports_per_signature();
(Ok(()), Some(nonce), lamports_per_signature)
} else {
error_counters.blockhash_not_found += 1;
(Err(TransactionError::BlockhashNotFound), None)
(Err(TransactionError::BlockhashNotFound), None, None)
}
}

Expand All @@ -4508,16 +4514,16 @@ impl Bank {
sanitized_txs
.iter()
.zip(lock_results)
.map(|(sanitized_tx, (lock_result, nonce))| {
.map(|(sanitized_tx, (lock_result, nonce, lamports))| {
let sanitized_tx = sanitized_tx.borrow();
if lock_result.is_ok()
&& self.is_transaction_already_processed(sanitized_tx, &rcache)
{
error_counters.already_processed += 1;
return (Err(TransactionError::AlreadyProcessed), None);
return (Err(TransactionError::AlreadyProcessed), None, None);
}

(lock_result, nonce)
(lock_result, nonce, lamports)
})
.collect()
}
Expand Down Expand Up @@ -5075,19 +5081,11 @@ impl Bank {
txs: &[SanitizedTransaction],
lock_results: &mut [TransactionCheckResult],
program_owners: &'a [Pubkey],
hash_queue: &BlockhashQueue,
) -> HashMap<Pubkey, (&'a Pubkey, u64)> {
let mut result: HashMap<Pubkey, (&'a Pubkey, u64)> = HashMap::new();
lock_results.iter_mut().zip(txs).for_each(|etx| {
if let ((Ok(()), nonce), tx) = etx {
if nonce
.as_ref()
.map(|nonce| nonce.lamports_per_signature())
.unwrap_or_else(|| {
hash_queue.get_lamports_per_signature(tx.message().recent_blockhash())
})
.is_some()
{
if let ((Ok(()), _nonce, lamports_per_signature), tx) = etx {
if lamports_per_signature.is_some() {
tx.message()
.account_keys()
.iter()
Expand All @@ -5113,7 +5111,7 @@ impl Bank {
// If the transaction's nonce account was not valid, and blockhash is not found,
// the transaction will fail to process. Let's not load any programs from the
// transaction, and update the status of the transaction.
*etx.0 = (Err(TransactionError::BlockhashNotFound), None);
*etx.0 = (Err(TransactionError::BlockhashNotFound), None, None);
}
}
});
Expand Down Expand Up @@ -5340,7 +5338,6 @@ impl Bank {
sanitized_txs,
check_results,
PROGRAM_OWNERS,
&self.blockhash_queue.read().unwrap(),
);
let native_loader = native_loader::id();
for builtin_program in self.builtin_programs.iter() {
Expand All @@ -5357,7 +5354,6 @@ impl Bank {
&self.ancestors,
sanitized_txs,
check_results,
&self.blockhash_queue.read().unwrap(),
error_counters,
&self.rent_collector,
&self.feature_set,
Expand Down
16 changes: 3 additions & 13 deletions runtime/src/bank/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10989,8 +10989,7 @@ fn test_rent_state_list_len() {
&bank.accounts().accounts_db,
&bank.ancestors,
&[sanitized_tx.clone()],
&[(Ok(()), None)],
&bank.blockhash_queue.read().unwrap(),
&[(Ok(()), None, Some(0))],
&mut error_counters,
&bank.rent_collector,
&bank.feature_set,
Expand Down Expand Up @@ -13723,16 +13722,13 @@ fn test_filter_executable_program_accounts() {
&AccountSharedData::new(40, 1, &program2_pubkey),
);

let mut hash_queue = BlockhashQueue::new(100);

let tx1 = Transaction::new_with_compiled_instructions(
&[&keypair1],
&[non_program_pubkey1],
Hash::new_unique(),
vec![account1_pubkey, account2_pubkey, account3_pubkey],
vec![CompiledInstruction::new(1, &(), vec![0])],
);
hash_queue.register_hash(&tx1.message().recent_blockhash, 0);
let sanitized_tx1 = SanitizedTransaction::from_transaction_for_tests(tx1);

let tx2 = Transaction::new_with_compiled_instructions(
Expand All @@ -13742,17 +13738,15 @@ fn test_filter_executable_program_accounts() {
vec![account4_pubkey, account3_pubkey, account2_pubkey],
vec![CompiledInstruction::new(1, &(), vec![0])],
);
hash_queue.register_hash(&tx2.message().recent_blockhash, 0);
let sanitized_tx2 = SanitizedTransaction::from_transaction_for_tests(tx2);

let ancestors = vec![(0, 0)].into_iter().collect();
let owners = &[program1_pubkey, program2_pubkey];
let programs = bank.filter_executable_program_accounts(
&ancestors,
&[sanitized_tx1, sanitized_tx2],
&mut [(Ok(()), None), (Ok(()), None)],
&mut [(Ok(()), None, Some(0)), (Ok(()), None, Some(0))],
owners,
&hash_queue,
);

// The result should contain only account3_pubkey, and account4_pubkey as the program accounts
Expand Down Expand Up @@ -13822,16 +13816,13 @@ fn test_filter_executable_program_accounts_invalid_blockhash() {
&AccountSharedData::new(40, 1, &program2_pubkey),
);

let mut hash_queue = BlockhashQueue::new(100);

let tx1 = Transaction::new_with_compiled_instructions(
&[&keypair1],
&[non_program_pubkey1],
Hash::new_unique(),
vec![account1_pubkey, account2_pubkey, account3_pubkey],
vec![CompiledInstruction::new(1, &(), vec![0])],
);
hash_queue.register_hash(&tx1.message().recent_blockhash, 0);
let sanitized_tx1 = SanitizedTransaction::from_transaction_for_tests(tx1);

let tx2 = Transaction::new_with_compiled_instructions(
Expand All @@ -13846,13 +13837,12 @@ fn test_filter_executable_program_accounts_invalid_blockhash() {

let ancestors = vec![(0, 0)].into_iter().collect();
let owners = &[program1_pubkey, program2_pubkey];
let mut lock_results = vec![(Ok(()), None), (Ok(()), None)];
let mut lock_results = vec![(Ok(()), None, Some(0)), (Ok(()), None, None)];
let programs = bank.filter_executable_program_accounts(
&ancestors,
&[sanitized_tx1, sanitized_tx2],
&mut lock_results,
owners,
&hash_queue,
);

// The result should contain only account3_pubkey as the program accounts
Expand Down
26 changes: 6 additions & 20 deletions runtime/src/svm/account_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ use {
accounts::{LoadedTransaction, TransactionLoadResult, TransactionRent},
accounts_db::AccountsDb,
ancestors::Ancestors,
blockhash_queue::BlockhashQueue,
nonce_info::{NonceFull, NonceInfo},
nonce_info::NonceFull,
rent_collector::{RentCollector, RENT_EXEMPT_RENT_EPOCH},
rent_debits::RentDebits,
transaction_error_metrics::TransactionErrorMetrics,
Expand Down Expand Up @@ -45,7 +44,6 @@ pub(crate) fn load_accounts(
ancestors: &Ancestors,
txs: &[SanitizedTransaction],
lock_results: &[TransactionCheckResult],
hash_queue: &BlockhashQueue,
error_counters: &mut TransactionErrorMetrics,
rent_collector: &RentCollector,
feature_set: &FeatureSet,
Expand All @@ -59,17 +57,11 @@ pub(crate) fn load_accounts(
txs.iter()
.zip(lock_results)
.map(|etx| match etx {
(tx, (Ok(()), nonce)) => {
let lamports_per_signature = nonce
.as_ref()
.map(|nonce| nonce.lamports_per_signature())
.unwrap_or_else(|| {
hash_queue.get_lamports_per_signature(tx.message().recent_blockhash())
});
(tx, (Ok(()), nonce, lamports_per_signature)) => {
let fee = if let Some(lamports_per_signature) = lamports_per_signature {
fee_structure.calculate_fee(
tx.message(),
lamports_per_signature,
*lamports_per_signature,
&process_compute_budget_instructions(
tx.message().program_instructions_iter(),
)
Expand Down Expand Up @@ -118,7 +110,7 @@ pub(crate) fn load_accounts(

(Ok(loaded_transaction), nonce)
}
(_, (Err(e), _nonce)) => (Err(e.clone()), None),
(_, (Err(e), _nonce, _lamports_per_signature)) => (Err(e.clone()), None),
})
.collect()
}
Expand Down Expand Up @@ -525,8 +517,6 @@ mod tests {
feature_set: &FeatureSet,
fee_structure: &FeeStructure,
) -> Vec<TransactionLoadResult> {
let mut hash_queue = BlockhashQueue::new(100);
hash_queue.register_hash(&tx.message().recent_blockhash, lamports_per_signature);
let accounts_db = AccountsDb::new_single_for_tests();
let accounts = Accounts::new(Arc::new(accounts_db));
for ka in ka.iter() {
Expand All @@ -539,8 +529,7 @@ mod tests {
&accounts.accounts_db,
&ancestors,
&[sanitized_tx],
&[(Ok(()), None)],
&hash_queue,
&[(Ok(()), None, Some(lamports_per_signature))],
error_counters,
rent_collector,
feature_set,
Expand Down Expand Up @@ -1008,17 +997,14 @@ mod tests {
) -> Vec<TransactionLoadResult> {
let tx = SanitizedTransaction::from_transaction_for_tests(tx);
let rent_collector = RentCollector::default();
let mut hash_queue = BlockhashQueue::new(100);
hash_queue.register_hash(tx.message().recent_blockhash(), 10);

let ancestors = vec![(0, 0)].into_iter().collect();
let mut error_counters = TransactionErrorMetrics::default();
load_accounts(
&accounts.accounts_db,
&ancestors,
&[tx],
&[(Ok(()), None)],
&hash_queue,
&[(Ok(()), None, Some(10))],
&mut error_counters,
&rent_collector,
&FeatureSet::all_enabled(),
Expand Down

0 comments on commit 0d117d4

Please sign in to comment.