Move transaction sanitization earlier in the pipeline (#18655)

* Move transaction sanitization earlier in the pipeline

* Renamed HashedTransaction to SanitizedTransaction

* Implement deref for sanitized transaction

* bring back process_transactions test method

* Use sanitized transactions for cost model calculation
This commit is contained in:
Justin Starry
2021-07-15 22:51:27 -05:00
committed by GitHub
parent c03490b24a
commit d166b9856a
21 changed files with 448 additions and 369 deletions

View File

@ -27,7 +27,6 @@ use solana_runtime::{
transaction_batch::TransactionBatch,
vote_sender_types::ReplayVoteSender,
};
use solana_sdk::hashed_transaction::HashedTransaction;
use solana_sdk::{
clock::{
Slot, DEFAULT_TICKS_PER_SLOT, MAX_PROCESSING_AGE, MAX_TRANSACTION_FORWARDING_DELAY,
@ -35,6 +34,7 @@ use solana_sdk::{
},
message::Message,
pubkey::Pubkey,
sanitized_transaction::SanitizedTransaction,
short_vec::decode_shortu16_len,
signature::Signature,
timing::{duration_as_ms, timestamp},
@ -863,11 +863,11 @@ impl BankingStage {
record_time.stop();
let mut commit_time = Measure::start("commit_time");
let hashed_txs = batch.hashed_transactions();
let sanitized_txs = batch.sanitized_transactions();
let num_to_commit = num_to_commit.unwrap();
if num_to_commit != 0 {
let tx_results = bank.commit_transactions(
hashed_txs,
sanitized_txs,
&mut loaded_accounts,
&results,
tx_count,
@ -875,7 +875,7 @@ impl BankingStage {
&mut execute_timings,
);
bank_utils::find_and_send_votes(hashed_txs, &tx_results, Some(gossip_vote_sender));
bank_utils::find_and_send_votes(sanitized_txs, &tx_results, Some(gossip_vote_sender));
if let Some(transaction_status_sender) = transaction_status_sender {
let txs = batch.transactions_iter().cloned().collect();
let post_balances = bank.collect_balances(batch);
@ -902,7 +902,7 @@ impl BankingStage {
load_execute_time.as_us(),
record_time.as_us(),
commit_time.as_us(),
hashed_txs.len(),
sanitized_txs.len(),
);
debug!(
@ -915,7 +915,7 @@ impl BankingStage {
pub fn process_and_record_transactions(
bank: &Arc<Bank>,
txs: &[HashedTransaction],
txs: &[SanitizedTransaction],
poh: &TransactionRecorder,
chunk_offset: usize,
transaction_status_sender: Option<TransactionStatusSender>,
@ -924,7 +924,7 @@ impl BankingStage {
let mut lock_time = Measure::start("lock_time");
// Once accounts are locked, other threads cannot encode transactions that will modify the
// same account state
let batch = bank.prepare_hashed_batch(txs);
let batch = bank.prepare_sanitized_batch(txs);
lock_time.stop();
let (result, mut retryable_txs) = Self::process_and_record_transactions_locked(
@ -959,7 +959,7 @@ impl BankingStage {
fn process_transactions(
bank: &Arc<Bank>,
bank_creation_time: &Instant,
transactions: &[HashedTransaction],
transactions: &[SanitizedTransaction],
poh: &TransactionRecorder,
transaction_status_sender: Option<TransactionStatusSender>,
gossip_vote_sender: &ReplayVoteSender,
@ -1062,7 +1062,7 @@ impl BankingStage {
libsecp256k1_0_5_upgrade_enabled: bool,
cost_tracker: &Arc<RwLock<CostTracker>>,
banking_stage_stats: &BankingStageStats,
) -> (Vec<HashedTransaction<'static>>, Vec<usize>, Vec<usize>) {
) -> (Vec<SanitizedTransaction<'static>>, Vec<usize>, Vec<usize>) {
let mut retryable_transaction_packet_indexes: Vec<usize> = vec![];
let verified_transactions_with_packet_indexes: Vec<_> = transaction_indexes
@ -1072,6 +1072,9 @@ impl BankingStage {
let tx: Transaction = limited_deserialize(&p.data[0..p.meta.size]).ok()?;
tx.verify_precompiles(libsecp256k1_0_5_upgrade_enabled)
.ok()?;
let message_bytes = Self::packet_message(p)?;
let message_hash = Message::hash_raw_message(message_bytes);
let tx = SanitizedTransaction::try_create(Cow::Owned(tx), message_hash).ok()?;
Some((tx, *tx_index))
})
.collect();
@ -1081,7 +1084,7 @@ impl BankingStage {
);
let mut cost_tracker_check_time = Measure::start("cost_tracker_check_time");
let filtered_transactions_with_packet_indexes: Vec<_> = {
let (filtered_transactions, filter_transaction_packet_indexes) = {
let cost_tracker_readonly = cost_tracker.read().unwrap();
verified_transactions_with_packet_indexes
.into_iter()
@ -1094,24 +1097,10 @@ impl BankingStage {
}
Some((tx, tx_index))
})
.collect()
.unzip()
};
cost_tracker_check_time.stop();
let (filtered_transactions, filter_transaction_packet_indexes) =
filtered_transactions_with_packet_indexes
.into_iter()
.filter_map(|(tx, tx_index)| {
let p = &msgs.packets[tx_index];
let message_bytes = Self::packet_message(p)?;
let message_hash = Message::hash_raw_message(message_bytes);
Some((
HashedTransaction::new(Cow::Owned(tx), message_hash),
tx_index,
))
})
.unzip();
banking_stage_stats
.cost_tracker_check_elapsed
.fetch_add(cost_tracker_check_time.as_us(), Ordering::Relaxed);
@ -1130,7 +1119,7 @@ impl BankingStage {
/// * `pending_indexes` - identifies which indexes in the `transactions` list are still pending
fn filter_pending_packets_from_pending_txs(
bank: &Arc<Bank>,
transactions: &[HashedTransaction],
transactions: &[SanitizedTransaction],
transaction_to_packet_indexes: &[usize],
pending_indexes: &[usize],
) -> Vec<usize> {
@ -1218,17 +1207,7 @@ impl BankingStage {
let mut cost_tracking_time = Measure::start("cost_tracking_time");
transactions.iter().enumerate().for_each(|(index, tx)| {
if unprocessed_tx_indexes.iter().all(|&i| i != index) {
cost_tracker
.write()
.unwrap()
.add_transaction_cost(tx.transaction())
.unwrap_or_else(|err| {
warn!(
"failed to track transaction cost, err {:?}, tx {:?}",
err,
tx.transaction()
)
});
cost_tracker.write().unwrap().add_transaction_cost(tx);
}
});
cost_tracking_time.stop();
@ -1602,6 +1581,7 @@ mod tests {
};
use solana_transaction_status::TransactionWithStatusMeta;
use std::{
convert::TryInto,
net::SocketAddr,
path::Path,
sync::{
@ -1817,7 +1797,7 @@ mod tests {
if !entries.is_empty() {
blockhash = entries.last().unwrap().hash;
for entry in entries {
bank.process_transactions(&entry.transactions)
bank.process_transactions(entry.transactions.iter())
.iter()
.for_each(|x| assert_eq!(*x, Ok(())));
}
@ -1931,7 +1911,7 @@ mod tests {
let bank = Bank::new_no_wallclock_throttle(&genesis_config);
for entry in &entries {
bank.process_transactions(&entry.transactions)
bank.process_transactions(entry.transactions.iter())
.iter()
.for_each(|x| assert_eq!(*x, Ok(())));
}
@ -2223,7 +2203,8 @@ mod tests {
let transactions =
vec![
system_transaction::transfer(&mint_keypair, &pubkey, 1, genesis_config.hash())
.into(),
.try_into()
.unwrap(),
];
let start = Arc::new(Instant::now());
@ -2292,7 +2273,8 @@ mod tests {
2,
genesis_config.hash(),
)
.into()];
.try_into()
.unwrap()];
assert_matches!(
BankingStage::process_and_record_transactions(
@ -2353,8 +2335,12 @@ mod tests {
let pubkey1 = solana_sdk::pubkey::new_rand();
let transactions = vec![
system_transaction::transfer(&mint_keypair, &pubkey, 1, genesis_config.hash()).into(),
system_transaction::transfer(&mint_keypair, &pubkey1, 1, genesis_config.hash()).into(),
system_transaction::transfer(&mint_keypair, &pubkey, 1, genesis_config.hash())
.try_into()
.unwrap(),
system_transaction::transfer(&mint_keypair, &pubkey1, 1, genesis_config.hash())
.try_into()
.unwrap(),
];
let start = Arc::new(Instant::now());
@ -2467,7 +2453,8 @@ mod tests {
let transactions =
vec![
system_transaction::transfer(&mint_keypair, &pubkey, 1, genesis_config.hash())
.into(),
.try_into()
.unwrap(),
];
let ledger_path = get_tmp_ledger_path!();
@ -2544,7 +2531,11 @@ mod tests {
let entry_3 = next_entry(&entry_2.hash, 1, vec![fail_tx.clone()]);
let entries = vec![entry_1, entry_2, entry_3];
let transactions = vec![success_tx.into(), ix_error_tx.into(), fail_tx.into()];
let transactions = vec![
success_tx.try_into().unwrap(),
ix_error_tx.try_into().unwrap(),
fail_tx.try_into().unwrap(),
];
bank.transfer(4, &mint_keypair, &keypair1.pubkey()).unwrap();
let start = Arc::new(Instant::now());

View File

@ -9,7 +9,7 @@
//!
use crate::execute_cost_table::ExecuteCostTable;
use log::*;
use solana_sdk::{pubkey::Pubkey, transaction::Transaction};
use solana_sdk::{pubkey::Pubkey, sanitized_transaction::SanitizedTransaction};
use std::collections::HashMap;
// Guestimated from mainnet-beta data, sigver averages 1us, average read 7us and average write 25us
@ -126,14 +126,11 @@ impl CostModel {
);
}
pub fn calculate_cost(
&mut self,
transaction: &Transaction,
) -> Result<&TransactionCost, CostModelError> {
pub fn calculate_cost(&mut self, transaction: &SanitizedTransaction) -> &TransactionCost {
self.transaction_cost.reset();
// calculate transaction exeution cost
self.transaction_cost.execution_cost = self.find_transaction_cost(transaction)?;
self.transaction_cost.execution_cost = self.find_transaction_cost(transaction);
// calculate account access cost
let message = transaction.message();
@ -159,7 +156,7 @@ impl CostModel {
"transaction {:?} has cost {:?}",
transaction, self.transaction_cost
);
Ok(&self.transaction_cost)
&self.transaction_cost
}
// To update or insert instruction cost to table.
@ -194,15 +191,10 @@ impl CostModel {
}
}
fn find_transaction_cost(&self, transaction: &Transaction) -> Result<u64, CostModelError> {
fn find_transaction_cost(&self, transaction: &SanitizedTransaction) -> u64 {
let mut cost: u64 = 0;
for instruction in &transaction.message().instructions {
// The Transaction may not be sanitized at this point
if instruction.program_id_index as usize >= transaction.message().account_keys.len() {
return Err(CostModelError::InvalidTransaction);
}
let program_id =
transaction.message().account_keys[instruction.program_id_index as usize];
let instruction_cost = self.find_instruction_cost(&program_id);
@ -213,7 +205,7 @@ impl CostModel {
);
cost += instruction_cost;
}
Ok(cost)
cost
}
}
@ -232,8 +224,10 @@ mod tests {
signature::{Keypair, Signer},
system_instruction::{self},
system_program, system_transaction,
transaction::Transaction,
};
use std::{
convert::{TryFrom, TryInto},
str::FromStr,
sync::{Arc, RwLock},
thread::{self, JoinHandle},
@ -279,8 +273,10 @@ mod tests {
let (mint_keypair, start_hash) = test_setup();
let keypair = Keypair::new();
let simple_transaction =
system_transaction::transfer(&mint_keypair, &keypair.pubkey(), 2, start_hash);
let simple_transaction: SanitizedTransaction =
system_transaction::transfer(&mint_keypair, &keypair.pubkey(), 2, start_hash)
.try_into()
.unwrap();
debug!(
"system_transaction simple_transaction {:?}",
simple_transaction
@ -295,7 +291,7 @@ mod tests {
.unwrap();
assert_eq!(
expected_cost,
testee.find_transaction_cost(&simple_transaction).unwrap()
testee.find_transaction_cost(&simple_transaction)
);
}
@ -308,7 +304,9 @@ mod tests {
let instructions =
system_instruction::transfer_many(&mint_keypair.pubkey(), &[(key1, 1), (key2, 1)]);
let message = Message::new(&instructions, Some(&mint_keypair.pubkey()));
let tx = Transaction::new(&[&mint_keypair], message, start_hash);
let tx: SanitizedTransaction = Transaction::new(&[&mint_keypair], message, start_hash)
.try_into()
.unwrap();
debug!("many transfer transaction {:?}", tx);
// expected cost for two system transfer instructions
@ -319,7 +317,7 @@ mod tests {
testee
.upsert_instruction_cost(&system_program::id(), program_cost)
.unwrap();
assert_eq!(expected_cost, testee.find_transaction_cost(&tx).unwrap());
assert_eq!(expected_cost, testee.find_transaction_cost(&tx));
}
#[test]
@ -335,17 +333,19 @@ mod tests {
CompiledInstruction::new(3, &(), vec![0, 1]),
CompiledInstruction::new(4, &(), vec![0, 2]),
];
let tx = Transaction::new_with_compiled_instructions(
let tx: SanitizedTransaction = Transaction::new_with_compiled_instructions(
&[&mint_keypair],
&[key1, key2],
start_hash,
vec![prog1, prog2],
instructions,
);
)
.try_into()
.unwrap();
debug!("many random transaction {:?}", tx);
let testee = CostModel::default();
let result = testee.find_transaction_cost(&tx).unwrap();
let result = testee.find_transaction_cost(&tx);
// expected cost for two random/unknown program is
let expected_cost = testee.instruction_execution_cost_table.get_mode() * 2;
@ -365,16 +365,18 @@ mod tests {
CompiledInstruction::new(4, &(), vec![0, 2]),
CompiledInstruction::new(5, &(), vec![1, 3]),
];
let tx = Transaction::new_with_compiled_instructions(
let tx: SanitizedTransaction = Transaction::new_with_compiled_instructions(
&[&signer1, &signer2],
&[key1, key2],
Hash::new_unique(),
vec![prog1, prog2],
instructions,
);
)
.try_into()
.unwrap();
let mut cost_model = CostModel::default();
let tx_cost = cost_model.calculate_cost(&tx).unwrap();
let tx_cost = cost_model.calculate_cost(&tx);
assert_eq!(2 + 2, tx_cost.writable_accounts.len());
assert_eq!(signer1.pubkey(), tx_cost.writable_accounts[0]);
assert_eq!(signer2.pubkey(), tx_cost.writable_accounts[1]);
@ -404,8 +406,10 @@ mod tests {
#[test]
fn test_cost_model_calculate_cost() {
let (mint_keypair, start_hash) = test_setup();
let tx =
system_transaction::transfer(&mint_keypair, &Keypair::new().pubkey(), 2, start_hash);
let tx: SanitizedTransaction =
system_transaction::transfer(&mint_keypair, &Keypair::new().pubkey(), 2, start_hash)
.try_into()
.unwrap();
let expected_account_cost = SIGNED_WRITABLE_ACCOUNT_ACCESS_COST
+ NON_SIGNED_WRITABLE_ACCOUNT_ACCESS_COST
@ -416,7 +420,7 @@ mod tests {
cost_model
.upsert_instruction_cost(&system_program::id(), expected_execution_cost)
.unwrap();
let tx_cost = cost_model.calculate_cost(&tx).unwrap();
let tx_cost = cost_model.calculate_cost(&tx);
assert_eq!(expected_account_cost, tx_cost.account_access_cost);
assert_eq!(expected_execution_cost, tx_cost.execution_cost);
assert_eq!(2, tx_cost.writable_accounts.len());
@ -452,13 +456,16 @@ mod tests {
CompiledInstruction::new(3, &(), vec![0, 1]),
CompiledInstruction::new(4, &(), vec![0, 2]),
];
let tx = Arc::new(Transaction::new_with_compiled_instructions(
&[&mint_keypair],
&[key1, key2],
start_hash,
vec![prog1, prog2],
instructions,
));
let tx = Arc::new(
SanitizedTransaction::try_from(Transaction::new_with_compiled_instructions(
&[&mint_keypair],
&[key1, key2],
start_hash,
vec![prog1, prog2],
instructions,
))
.unwrap(),
);
let number_threads = 10;
let expected_account_cost = SIGNED_WRITABLE_ACCOUNT_ACCESS_COST
@ -484,7 +491,7 @@ mod tests {
} else {
thread::spawn(move || {
let mut cost_model = cost_model.write().unwrap();
let tx_cost = cost_model.calculate_cost(&tx).unwrap();
let tx_cost = cost_model.calculate_cost(&tx);
assert_eq!(3, tx_cost.writable_accounts.len());
assert_eq!(expected_account_cost, tx_cost.account_access_cost);
})

View File

@ -5,7 +5,7 @@
//! - add_transaction_cost(&tx), mutable function to accumulate `tx` cost to tracker.
//!
use crate::cost_model::{CostModel, CostModelError, TransactionCost};
use solana_sdk::{clock::Slot, pubkey::Pubkey, transaction::Transaction};
use solana_sdk::{clock::Slot, pubkey::Pubkey, sanitized_transaction::SanitizedTransaction};
use std::{
collections::HashMap,
sync::{Arc, RwLock},
@ -43,21 +43,21 @@ impl CostTracker {
}
}
pub fn would_transaction_fit(&self, transaction: &Transaction) -> Result<(), CostModelError> {
pub fn would_transaction_fit(
&self,
transaction: &SanitizedTransaction,
) -> Result<(), CostModelError> {
let mut cost_model = self.cost_model.write().unwrap();
let tx_cost = cost_model.calculate_cost(transaction)?;
let tx_cost = cost_model.calculate_cost(transaction);
self.would_fit(
&tx_cost.writable_accounts,
&(tx_cost.account_access_cost + tx_cost.execution_cost),
)
}
pub fn add_transaction_cost(
&mut self,
transaction: &Transaction,
) -> Result<(), CostModelError> {
pub fn add_transaction_cost(&mut self, transaction: &SanitizedTransaction) {
let mut cost_model = self.cost_model.write().unwrap();
let tx_cost = cost_model.calculate_cost(transaction)?;
let tx_cost = cost_model.calculate_cost(transaction);
let cost = tx_cost.account_access_cost + tx_cost.execution_cost;
for account_key in tx_cost.writable_accounts.iter() {
*self
@ -66,7 +66,6 @@ impl CostTracker {
.or_insert(0) += cost;
}
self.block_cost += cost;
Ok(())
}
pub fn reset_if_new_bank(&mut self, slot: Slot) {