Skip to content

Commit

Permalink
Use reserved account keys list to restrict tx write locks (#541)
Browse files Browse the repository at this point in the history
* Plumb through reserved account keys set

* Plumb through tests
  • Loading branch information
jstarry authored Apr 13, 2024
1 parent cb13b39 commit 09241ae
Show file tree
Hide file tree
Showing 34 changed files with 355 additions and 137 deletions.
6 changes: 5 additions & 1 deletion banks-server/src/banks_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ fn simulate_transaction(
MessageHash::Compute,
Some(false), // is_simple_vote_tx
bank,
bank.get_reserved_account_keys(),
) {
Err(err) => {
return BanksTransactionResultWithSimulation {
Expand Down Expand Up @@ -332,6 +333,7 @@ impl Banks for BanksServer {
MessageHash::Compute,
Some(false), // is_simple_vote_tx
bank.as_ref(),
bank.get_reserved_account_keys(),
) {
Ok(tx) => tx,
Err(err) => return Some(Err(err)),
Expand Down Expand Up @@ -417,7 +419,9 @@ impl Banks for BanksServer {
commitment: CommitmentLevel,
) -> Option<u64> {
let bank = self.bank(commitment);
let sanitized_message = SanitizedMessage::try_from_legacy_message(message).ok()?;
let sanitized_message =
SanitizedMessage::try_from_legacy_message(message, bank.get_reserved_account_keys())
.ok()?;
bank.get_fee_for_message(&sanitized_message)
}
}
Expand Down
2 changes: 2 additions & 0 deletions core/src/banking_stage/consumer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -868,6 +868,7 @@ mod tests {
nonce_account::verify_nonce_account,
poh_config::PohConfig,
pubkey::Pubkey,
reserved_account_keys::ReservedAccountKeys,
signature::Keypair,
signer::Signer,
system_instruction, system_program, system_transaction,
Expand Down Expand Up @@ -2026,6 +2027,7 @@ mod tests {
MessageHash::Compute,
Some(false),
bank.as_ref(),
&ReservedAccountKeys::empty_key_set(),
)
.unwrap();

Expand Down
5 changes: 4 additions & 1 deletion core/src/banking_stage/immutable_deserialized_packet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use {
feature_set,
hash::Hash,
message::Message,
pubkey::Pubkey,
sanitize::SanitizeError,
saturating_add_assign,
short_vec::decode_shortu16_len,
Expand All @@ -15,7 +16,7 @@ use {
VersionedTransaction,
},
},
std::{cmp::Ordering, mem::size_of, sync::Arc},
std::{cmp::Ordering, collections::HashSet, mem::size_of, sync::Arc},
thiserror::Error,
};

Expand Down Expand Up @@ -123,6 +124,7 @@ impl ImmutableDeserializedPacket {
feature_set: &Arc<feature_set::FeatureSet>,
votes_only: bool,
address_loader: impl AddressLoader,
reserved_account_keys: &HashSet<Pubkey>,
) -> Option<SanitizedTransaction> {
if votes_only && !self.is_simple_vote() {
return None;
Expand All @@ -132,6 +134,7 @@ impl ImmutableDeserializedPacket {
*self.message_hash(),
self.is_simple_vote(),
address_loader,
reserved_account_keys,
)
.ok()?;
tx.verify_precompiles(feature_set).ok()?;
Expand Down
1 change: 1 addition & 0 deletions core/src/banking_stage/latest_unprocessed_votes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ impl LatestUnprocessedVotes {
&bank.feature_set,
bank.vote_only_bank(),
bank.as_ref(),
bank.get_reserved_account_keys(),
)
{
if forward_packet_batches_by_accounts.try_add_packet(
Expand Down
1 change: 1 addition & 0 deletions core/src/banking_stage/read_write_account_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ mod tests {
MessageHash::Compute,
Some(false),
bank,
bank.get_reserved_account_keys(),
)
.unwrap()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,12 @@ impl SchedulerController {
let (transactions, fee_budget_limits_vec): (Vec<_>, Vec<_>) = chunk
.iter()
.filter_map(|packet| {
packet.build_sanitized_transaction(feature_set, vote_only, bank.as_ref())
packet.build_sanitized_transaction(
feature_set,
vote_only,
bank.as_ref(),
bank.get_reserved_account_keys(),
)
})
.inspect(|_| saturating_add_assign!(post_sanitization_count, 1))
.filter(|tx| {
Expand Down
7 changes: 7 additions & 0 deletions core/src/banking_stage/unprocessed_packet_batches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,7 @@ mod tests {
solana_sdk::{
compute_budget::ComputeBudgetInstruction,
message::Message,
reserved_account_keys::ReservedAccountKeys,
signature::{Keypair, Signer},
system_instruction, system_transaction,
transaction::{SimpleAddressLoader, Transaction},
Expand Down Expand Up @@ -490,6 +491,7 @@ mod tests {
&Arc::new(FeatureSet::default()),
votes_only,
SimpleAddressLoader::Disabled,
&ReservedAccountKeys::empty_key_set(),
)
});
assert_eq!(2, txs.count());
Expand All @@ -500,6 +502,7 @@ mod tests {
&Arc::new(FeatureSet::default()),
votes_only,
SimpleAddressLoader::Disabled,
&ReservedAccountKeys::empty_key_set(),
)
});
assert_eq!(0, txs.count());
Expand All @@ -519,6 +522,7 @@ mod tests {
&Arc::new(FeatureSet::default()),
votes_only,
SimpleAddressLoader::Disabled,
&ReservedAccountKeys::empty_key_set(),
)
});
assert_eq!(3, txs.count());
Expand All @@ -529,6 +533,7 @@ mod tests {
&Arc::new(FeatureSet::default()),
votes_only,
SimpleAddressLoader::Disabled,
&ReservedAccountKeys::empty_key_set(),
)
});
assert_eq!(2, txs.count());
Expand All @@ -548,6 +553,7 @@ mod tests {
&Arc::new(FeatureSet::default()),
votes_only,
SimpleAddressLoader::Disabled,
&ReservedAccountKeys::empty_key_set(),
)
});
assert_eq!(3, txs.count());
Expand All @@ -558,6 +564,7 @@ mod tests {
&Arc::new(FeatureSet::default()),
votes_only,
SimpleAddressLoader::Disabled,
&ReservedAccountKeys::empty_key_set(),
)
});
assert_eq!(3, txs.count());
Expand Down
17 changes: 13 additions & 4 deletions core/src/banking_stage/unprocessed_transaction_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,13 @@ fn consume_scan_should_process_packet(
}

// Try to sanitize the packet
let (maybe_sanitized_transaction, sanitization_time_us) = measure_us!(
packet.build_sanitized_transaction(&bank.feature_set, bank.vote_only_bank(), bank)
);
let (maybe_sanitized_transaction, sanitization_time_us) = measure_us!(packet
.build_sanitized_transaction(
&bank.feature_set,
bank.vote_only_bank(),
bank,
bank.get_reserved_account_keys(),
));

payload
.slot_metrics_tracker
Expand Down Expand Up @@ -770,7 +774,12 @@ impl ThreadLocalUnprocessedPackets {
.enumerate()
.filter_map(|(packet_index, deserialized_packet)| {
deserialized_packet
.build_sanitized_transaction(&bank.feature_set, bank.vote_only_bank(), bank)
.build_sanitized_transaction(
&bank.feature_set,
bank.vote_only_bank(),
bank,
bank.get_reserved_account_keys(),
)
.map(|transaction| (transaction, packet_index))
})
.unzip();
Expand Down
2 changes: 2 additions & 0 deletions cost-model/src/cost_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ mod tests {
crate::transaction_cost::*,
solana_sdk::{
hash::Hash,
reserved_account_keys::ReservedAccountKeys,
signature::{Keypair, Signer},
system_transaction,
transaction::{
Expand Down Expand Up @@ -401,6 +402,7 @@ mod tests {
MessageHash::Compute,
Some(true),
SimpleAddressLoader::Disabled,
&ReservedAccountKeys::empty_key_set(),
)
.unwrap();

Expand Down
3 changes: 3 additions & 0 deletions cost-model/src/transaction_cost.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ mod tests {
feature_set::FeatureSet,
hash::Hash,
message::SimpleAddressLoader,
reserved_account_keys::ReservedAccountKeys,
signer::keypair::Keypair,
transaction::{MessageHash, SanitizedTransaction, VersionedTransaction},
},
Expand Down Expand Up @@ -231,6 +232,7 @@ mod tests {
MessageHash::Compute,
Some(true),
SimpleAddressLoader::Disabled,
&ReservedAccountKeys::empty_key_set(),
)
.unwrap();

Expand All @@ -240,6 +242,7 @@ mod tests {
MessageHash::Compute,
Some(false),
SimpleAddressLoader::Disabled,
&ReservedAccountKeys::empty_key_set(),
)
.unwrap();

Expand Down
3 changes: 3 additions & 0 deletions entry/benches/entry_sigverify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use {
solana_perf::test_tx::test_tx,
solana_sdk::{
hash::Hash,
reserved_account_keys::ReservedAccountKeys,
transaction::{
Result, SanitizedTransaction, SimpleAddressLoader, TransactionVerificationMode,
VersionedTransaction,
Expand Down Expand Up @@ -41,6 +42,7 @@ fn bench_gpusigverify(bencher: &mut Bencher) {
message_hash,
None,
SimpleAddressLoader::Disabled,
&ReservedAccountKeys::empty_key_set(),
)
}?;

Expand Down Expand Up @@ -84,6 +86,7 @@ fn bench_cpusigverify(bencher: &mut Bencher) {
message_hash,
None,
SimpleAddressLoader::Disabled,
&ReservedAccountKeys::empty_key_set(),
)
}?;

Expand Down
2 changes: 2 additions & 0 deletions entry/src/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -982,6 +982,7 @@ mod tests {
solana_sdk::{
hash::{hash, Hash},
pubkey::Pubkey,
reserved_account_keys::ReservedAccountKeys,
signature::{Keypair, Signer},
system_transaction,
transaction::{
Expand Down Expand Up @@ -1084,6 +1085,7 @@ mod tests {
message_hash,
None,
SimpleAddressLoader::Disabled,
&ReservedAccountKeys::empty_key_set(),
)
}?;

Expand Down
7 changes: 6 additions & 1 deletion ledger-tool/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ use {
native_token::{lamports_to_sol, sol_to_lamports, Sol},
pubkey::Pubkey,
rent::Rent,
reserved_account_keys::ReservedAccountKeys,
shred_version::compute_shred_version,
stake::{self, state::StakeStateV2},
system_program,
Expand Down Expand Up @@ -461,6 +462,9 @@ fn compute_slot_cost(
let mut program_ids = HashMap::new();
let mut cost_tracker = CostTracker::default();

let feature_set = FeatureSet::all_enabled();
let reserved_account_keys = ReservedAccountKeys::new_all_activated();

for entry in entries {
num_transactions += entry.transactions.len();
entry
Expand All @@ -472,6 +476,7 @@ fn compute_slot_cost(
MessageHash::Compute,
None,
SimpleAddressLoader::Disabled,
&reserved_account_keys.active,
)
.map_err(|err| {
warn!("Failed to compute cost of transaction: {:?}", err);
Expand All @@ -481,7 +486,7 @@ fn compute_slot_cost(
.for_each(|transaction| {
num_programs += transaction.message().instructions().len();

let tx_cost = CostModel::calculate_cost(&transaction, &FeatureSet::all_enabled());
let tx_cost = CostModel::calculate_cost(&transaction, &feature_set);
let result = cost_tracker.try_add(&tx_cost);
if result.is_err() {
println!(
Expand Down
19 changes: 16 additions & 3 deletions programs/sbf/tests/programs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ use {
message::Message,
pubkey::Pubkey,
rent::Rent,
reserved_account_keys::ReservedAccountKeys,
signature::{Keypair, Signer},
system_program,
transaction::{SanitizedTransaction, Transaction, TransactionError},
Expand Down Expand Up @@ -201,7 +202,11 @@ fn execute_transactions(
}
.expect("lamports_per_signature must be available");
let fee = bank.get_fee_for_message_with_lamports_per_signature(
&SanitizedMessage::try_from_legacy_message(tx.message().clone()).unwrap(),
&SanitizedMessage::try_from_legacy_message(
tx.message().clone(),
&ReservedAccountKeys::empty_key_set(),
)
.unwrap(),
lamports_per_signature,
);

Expand Down Expand Up @@ -3706,7 +3711,11 @@ fn test_program_fees() {
Some(&mint_keypair.pubkey()),
);

let sanitized_message = SanitizedMessage::try_from_legacy_message(message.clone()).unwrap();
let sanitized_message = SanitizedMessage::try_from_legacy_message(
message.clone(),
&ReservedAccountKeys::empty_key_set(),
)
.unwrap();
let expected_normal_fee = fee_structure.calculate_fee(
&sanitized_message,
congestion_multiplier,
Expand All @@ -3730,7 +3739,11 @@ fn test_program_fees() {
],
Some(&mint_keypair.pubkey()),
);
let sanitized_message = SanitizedMessage::try_from_legacy_message(message.clone()).unwrap();
let sanitized_message = SanitizedMessage::try_from_legacy_message(
message.clone(),
&ReservedAccountKeys::empty_key_set(),
)
.unwrap();
let expected_prioritized_fee = fee_structure.calculate_fee(
&sanitized_message,
congestion_multiplier,
Expand Down
Loading

0 comments on commit 09241ae

Please sign in to comment.