Simplify replay vote tracking by using packet metadata (#21112)

This commit is contained in:
Justin Starry 2021-11-03 09:02:48 +00:00 committed by GitHub
parent e6280fc1fa
commit 140a5f633d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 43 additions and 47 deletions

View File

@ -1115,9 +1115,12 @@ impl BankingStage {
let tx: VersionedTransaction = limited_deserialize(&p.data[0..p.meta.size]).ok()?; let tx: VersionedTransaction = limited_deserialize(&p.data[0..p.meta.size]).ok()?;
let message_bytes = Self::packet_message(p)?; let message_bytes = Self::packet_message(p)?;
let message_hash = Message::hash_raw_message(message_bytes); let message_hash = Message::hash_raw_message(message_bytes);
let tx = SanitizedTransaction::try_create(tx, message_hash, |_| { let tx = SanitizedTransaction::try_create(
Err(TransactionError::UnsupportedVersion) tx,
}) message_hash,
Some(p.meta.is_simple_vote_tx),
|_| Err(TransactionError::UnsupportedVersion),
)
.ok()?; .ok()?;
tx.verify_precompiles(feature_set).ok()?; tx.verify_precompiles(feature_set).ok()?;
Some((tx, *tx_index)) Some((tx, *tx_index))

View File

@ -223,7 +223,7 @@ fn output_slot(
for transaction in entry.transactions { for transaction in entry.transactions {
let tx_signature = transaction.signatures[0]; let tx_signature = transaction.signatures[0];
let sanitize_result = let sanitize_result =
SanitizedTransaction::try_create(transaction, Hash::default(), |_| { SanitizedTransaction::try_create(transaction, Hash::default(), None, |_| {
Err(TransactionError::UnsupportedVersion) Err(TransactionError::UnsupportedVersion)
}); });
@ -779,7 +779,7 @@ fn compute_slot_cost(blockstore: &Blockstore, slot: Slot) -> Result<(), String>
.transactions .transactions
.into_iter() .into_iter()
.filter_map(|transaction| { .filter_map(|transaction| {
SanitizedTransaction::try_create(transaction, Hash::default(), |_| { SanitizedTransaction::try_create(transaction, Hash::default(), None, |_| {
Err(TransactionError::UnsupportedVersion) Err(TransactionError::UnsupportedVersion)
}) })
.map_err(|err| { .map_err(|err| {

View File

@ -4172,7 +4172,7 @@ where
fn sanitize_transaction(transaction: VersionedTransaction) -> Result<SanitizedTransaction> { fn sanitize_transaction(transaction: VersionedTransaction) -> Result<SanitizedTransaction> {
let message_hash = transaction.message.hash(); let message_hash = transaction.message.hash();
SanitizedTransaction::try_create(transaction, message_hash, |_| { SanitizedTransaction::try_create(transaction, message_hash, None, |_| {
Err(TransactionError::UnsupportedVersion) Err(TransactionError::UnsupportedVersion)
}) })
.map_err(|err| Error::invalid_params(format!("invalid transaction: {}", err))) .map_err(|err| Error::invalid_params(format!("invalid transaction: {}", err)))

View File

@ -489,7 +489,6 @@ pub type TransactionExecutionResult = (Result<()>, Option<NonceRollbackFull>);
pub struct TransactionResults { pub struct TransactionResults {
pub fee_collection_results: Vec<Result<()>>, pub fee_collection_results: Vec<Result<()>>,
pub execution_results: Vec<TransactionExecutionResult>, pub execution_results: Vec<TransactionExecutionResult>,
pub overwritten_vote_accounts: Vec<OverwrittenVoteAccount>,
pub rent_debits: Vec<RentDebits>, pub rent_debits: Vec<RentDebits>,
} }
pub struct TransactionSimulationResult { pub struct TransactionSimulationResult {
@ -502,11 +501,6 @@ pub struct TransactionBalancesSet {
pub pre_balances: TransactionBalances, pub pre_balances: TransactionBalances,
pub post_balances: TransactionBalances, pub post_balances: TransactionBalances,
} }
pub struct OverwrittenVoteAccount {
pub account: VoteAccount,
pub transaction_index: usize,
pub transaction_result_index: usize,
}
impl TransactionBalancesSet { impl TransactionBalancesSet {
pub fn new(pre_balances: TransactionBalances, post_balances: TransactionBalances) -> Self { pub fn new(pre_balances: TransactionBalances, post_balances: TransactionBalances) -> Self {
@ -3273,7 +3267,7 @@ impl Bank {
.into_iter() .into_iter()
.map(|tx| { .map(|tx| {
let message_hash = tx.message.hash(); let message_hash = tx.message.hash();
SanitizedTransaction::try_create(tx, message_hash, |_| { SanitizedTransaction::try_create(tx, message_hash, None, |_| {
Err(TransactionError::UnsupportedVersion) Err(TransactionError::UnsupportedVersion)
}) })
}) })
@ -4133,7 +4127,6 @@ impl Bank {
let rent_debits = self.collect_rent(executed, loaded_txs); let rent_debits = self.collect_rent(executed, loaded_txs);
let mut update_stakes_cache_time = Measure::start("update_stakes_cache_time"); let mut update_stakes_cache_time = Measure::start("update_stakes_cache_time");
let overwritten_vote_accounts =
self.update_stakes_cache(sanitized_txs, executed, loaded_txs); self.update_stakes_cache(sanitized_txs, executed, loaded_txs);
update_stakes_cache_time.stop(); update_stakes_cache_time.stop();
@ -4155,7 +4148,6 @@ impl Bank {
TransactionResults { TransactionResults {
fee_collection_results, fee_collection_results,
execution_results: executed.to_vec(), execution_results: executed.to_vec(),
overwritten_vote_accounts,
rent_debits, rent_debits,
} }
} }
@ -5486,7 +5478,7 @@ impl Bank {
tx.message.hash() tx.message.hash()
}; };
SanitizedTransaction::try_create(tx, message_hash, |_| { SanitizedTransaction::try_create(tx, message_hash, None, |_| {
Err(TransactionError::UnsupportedVersion) Err(TransactionError::UnsupportedVersion)
}) })
}?; }?;
@ -5721,8 +5713,7 @@ impl Bank {
txs: &[SanitizedTransaction], txs: &[SanitizedTransaction],
res: &[TransactionExecutionResult], res: &[TransactionExecutionResult],
loaded_txs: &[TransactionLoadResult], loaded_txs: &[TransactionLoadResult],
) -> Vec<OverwrittenVoteAccount> { ) {
let mut overwritten_vote_accounts = vec![];
for (i, ((raccs, _load_nonce_rollback), tx)) in loaded_txs.iter().zip(txs).enumerate() { for (i, ((raccs, _load_nonce_rollback), tx)) in loaded_txs.iter().zip(txs).enumerate() {
let (res, _res_nonce_rollback) = &res[i]; let (res, _res_nonce_rollback) = &res[i];
if res.is_err() || raccs.is_err() { if res.is_err() || raccs.is_err() {
@ -5736,24 +5727,15 @@ impl Bank {
.zip(loaded_transaction.accounts.iter()) .zip(loaded_transaction.accounts.iter())
.filter(|(_i, (_pubkey, account))| (Stakes::is_stake(account))) .filter(|(_i, (_pubkey, account))| (Stakes::is_stake(account)))
{ {
if let Some(old_vote_account) = self.stakes.write().unwrap().store( self.stakes.write().unwrap().store(
pubkey, pubkey,
account, account,
self.stakes_remove_delegation_if_inactive_enabled(), self.stakes_remove_delegation_if_inactive_enabled(),
) { );
// TODO: one of the indices is redundant.
overwritten_vote_accounts.push(OverwrittenVoteAccount {
account: old_vote_account,
transaction_index: i,
transaction_result_index: i,
});
} }
} }
} }
overwritten_vote_accounts
}
/// current stake delegations for this bank /// current stake delegations for this bank
pub fn cloned_stake_delegations(&self) -> HashMap<Pubkey, Delegation> { pub fn cloned_stake_delegations(&self) -> HashMap<Pubkey, Delegation> {
self.stakes.read().unwrap().stake_delegations().clone() self.stakes.read().unwrap().stake_delegations().clone()

View File

@ -35,21 +35,21 @@ pub fn find_and_send_votes(
vote_sender: Option<&ReplayVoteSender>, vote_sender: Option<&ReplayVoteSender>,
) { ) {
let TransactionResults { let TransactionResults {
execution_results, execution_results, ..
overwritten_vote_accounts,
..
} = tx_results; } = tx_results;
if let Some(vote_sender) = vote_sender { if let Some(vote_sender) = vote_sender {
for old_account in overwritten_vote_accounts { sanitized_txs.iter().zip(execution_results.iter()).for_each(
assert!(execution_results[old_account.transaction_result_index] |(tx, (result, _nonce_rollback))| {
.0 if tx.is_simple_vote_transaction() && result.is_ok() {
.is_ok()); if let Some(parsed_vote) =
let tx = &sanitized_txs[old_account.transaction_index]; vote_transaction::parse_sanitized_vote_transaction(tx)
if let Some(parsed_vote) = vote_transaction::parse_sanitized_vote_transaction(tx) { {
if parsed_vote.1.slots.last().is_some() { if parsed_vote.1.slots.last().is_some() {
let _ = vote_sender.send(parsed_vote); let _ = vote_sender.send(parsed_vote);
} }
} }
} }
},
);
} }
} }

View File

@ -196,7 +196,7 @@ impl Stakes {
pubkey: &Pubkey, pubkey: &Pubkey,
account: &AccountSharedData, account: &AccountSharedData,
remove_delegation_on_inactive: bool, remove_delegation_on_inactive: bool,
) -> Option<VoteAccount> { ) {
if solana_vote_program::check_id(account.owner()) { if solana_vote_program::check_id(account.owner()) {
// unconditionally remove existing at first; there is no dependent calculated state for // unconditionally remove existing at first; there is no dependent calculated state for
// votes, not like stakes (stake codepath maintains calculated stake value grouped by // votes, not like stakes (stake codepath maintains calculated stake value grouped by
@ -213,7 +213,6 @@ impl Stakes {
self.vote_accounts self.vote_accounts
.insert(*pubkey, (stake, VoteAccount::from(account.clone()))); .insert(*pubkey, (stake, VoteAccount::from(account.clone())));
} }
old.map(|(_, account)| account)
} else if stake::program::check_id(account.owner()) { } else if stake::program::check_id(account.owner()) {
// old_stake is stake lamports and voter_pubkey from the pre-store() version // old_stake is stake lamports and voter_pubkey from the pre-store() version
let old_stake = self.stake_delegations.get(pubkey).map(|delegation| { let old_stake = self.stake_delegations.get(pubkey).map(|delegation| {
@ -263,13 +262,11 @@ impl Stakes {
} else if let Some(delegation) = delegation { } else if let Some(delegation) = delegation {
self.stake_delegations.insert(*pubkey, delegation); self.stake_delegations.insert(*pubkey, delegation);
} }
None
} else { } else {
// there is no need to remove possibly existing Stakes cache entries with given // there is no need to remove possibly existing Stakes cache entries with given
// `pubkey` because this isn't possible, first of all. // `pubkey` because this isn't possible, first of all.
// Runtime always enforces an intermediary write of account.lamports == 0, // Runtime always enforces an intermediary write of account.lamports == 0,
// when not-System111-owned account.owner is swapped. // when not-System111-owned account.owner is swapped.
None
} }
} }

View File

@ -22,6 +22,7 @@ use {
pub struct SanitizedTransaction { pub struct SanitizedTransaction {
message: SanitizedMessage, message: SanitizedMessage,
message_hash: Hash, message_hash: Hash,
is_simple_vote_tx: bool,
signatures: Vec<Signature>, signatures: Vec<Signature>,
} }
@ -41,6 +42,7 @@ impl SanitizedTransaction {
pub fn try_create( pub fn try_create(
tx: VersionedTransaction, tx: VersionedTransaction,
message_hash: Hash, message_hash: Hash,
is_simple_vote_tx: Option<bool>,
address_mapper: impl Fn(&v0::Message) -> Result<MappedAddresses>, address_mapper: impl Fn(&v0::Message) -> Result<MappedAddresses>,
) -> Result<Self> { ) -> Result<Self> {
tx.sanitize()?; tx.sanitize()?;
@ -58,9 +60,15 @@ impl SanitizedTransaction {
return Err(TransactionError::AccountLoadedTwice); return Err(TransactionError::AccountLoadedTwice);
} }
let is_simple_vote_tx = is_simple_vote_tx.unwrap_or_else(|| {
let mut ix_iter = message.program_instructions_iter();
ix_iter.next().map(|(program_id, _ix)| program_id) == Some(&crate::vote::program::id())
});
Ok(Self { Ok(Self {
message, message,
message_hash, message_hash,
is_simple_vote_tx,
signatures, signatures,
}) })
} }
@ -76,6 +84,7 @@ impl SanitizedTransaction {
Self { Self {
message_hash: tx.message.hash(), message_hash: tx.message.hash(),
message: SanitizedMessage::Legacy(tx.message), message: SanitizedMessage::Legacy(tx.message),
is_simple_vote_tx: false,
signatures: tx.signatures, signatures: tx.signatures,
} }
} }
@ -106,6 +115,11 @@ impl SanitizedTransaction {
&self.message_hash &self.message_hash
} }
/// Returns true if this transaction is a simple vote
pub fn is_simple_vote_transaction(&self) -> bool {
self.is_simple_vote_tx
}
/// Convert this sanitized transaction into a versioned transaction for /// Convert this sanitized transaction into a versioned transaction for
/// recording in the ledger. /// recording in the ledger.
pub fn to_versioned_transaction(&self) -> VersionedTransaction { pub fn to_versioned_transaction(&self) -> VersionedTransaction {