Skip to content

Commit

Permalink
Plumb through reserved account keys set
Browse files Browse the repository at this point in the history
  • Loading branch information
jstarry committed Apr 12, 2024
1 parent bad8a10 commit 7713959
Show file tree
Hide file tree
Showing 16 changed files with 169 additions and 57 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
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
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
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
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
30 changes: 22 additions & 8 deletions rpc/src/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3657,7 +3657,11 @@ pub mod rpc_full {
min_context_slot,
})?;

let transaction = sanitize_transaction(unsanitized_tx, preflight_bank)?;
let transaction = sanitize_transaction(
unsanitized_tx,
preflight_bank,
preflight_bank.get_reserved_account_keys(),
)?;
let signature = *transaction.signature();

let mut last_valid_block_height = preflight_bank
Expand Down Expand Up @@ -3789,7 +3793,8 @@ pub mod rpc_full {
});
}

let transaction = sanitize_transaction(unsanitized_tx, bank)?;
let transaction =
sanitize_transaction(unsanitized_tx, bank, bank.get_reserved_account_keys())?;
if sig_verify {
verify_transaction(&transaction, &bank.feature_set)?;
}
Expand Down Expand Up @@ -4041,10 +4046,12 @@ pub mod rpc_full {
.map_err(|err| {
Error::invalid_params(format!("invalid transaction message: {err}"))
})?;
let sanitized_message = SanitizedMessage::try_new(sanitized_versioned_message, bank)
.map_err(|err| {
Error::invalid_params(format!("invalid transaction message: {err}"))
})?;
let sanitized_message = SanitizedMessage::try_new(
sanitized_versioned_message,
bank,
bank.get_reserved_account_keys(),
)
.map_err(|err| Error::invalid_params(format!("invalid transaction message: {err}")))?;
let fee = bank.get_fee_for_message(&sanitized_message);
Ok(new_response(bank, fee))
}
Expand Down Expand Up @@ -4623,9 +4630,16 @@ where
fn sanitize_transaction(
transaction: VersionedTransaction,
address_loader: impl AddressLoader,
reserved_account_keys: &HashSet<Pubkey>,
) -> Result<SanitizedTransaction> {
SanitizedTransaction::try_create(transaction, MessageHash::Compute, None, address_loader)
.map_err(|err| Error::invalid_params(format!("invalid transaction: {err}")))
SanitizedTransaction::try_create(
transaction,
MessageHash::Compute,
None,
address_loader,
reserved_account_keys,
)
.map_err(|err| Error::invalid_params(format!("invalid transaction: {err}")))
}

pub fn create_validator_exit(exit: Arc<AtomicBool>) -> Arc<RwLock<Exit>> {
Expand Down
4 changes: 4 additions & 0 deletions runtime-transaction/src/runtime_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@ use {
solana_sdk::{
hash::Hash,
message::{AddressLoader, SanitizedMessage, SanitizedVersionedMessage},
pubkey::Pubkey,
signature::Signature,
simple_vote_transaction_checker::is_simple_vote_transaction,
transaction::{Result, SanitizedVersionedTransaction},
},
std::collections::HashSet,
};

#[derive(Debug, Clone, Eq, PartialEq)]
Expand Down Expand Up @@ -101,12 +103,14 @@ impl RuntimeTransaction<SanitizedMessage> {
pub fn try_from(
statically_loaded_runtime_tx: RuntimeTransaction<SanitizedVersionedMessage>,
address_loader: impl AddressLoader,
reserved_account_keys: &HashSet<Pubkey>,
) -> Result<Self> {
let mut tx = Self {
signatures: statically_loaded_runtime_tx.signatures,
message: SanitizedMessage::try_new(
statically_loaded_runtime_tx.message,
address_loader,
reserved_account_keys,
)?,
meta: statically_loaded_runtime_tx.meta,
};
Expand Down
18 changes: 16 additions & 2 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3470,7 +3470,15 @@ impl Bank {
pub fn prepare_entry_batch(&self, txs: Vec<VersionedTransaction>) -> Result<TransactionBatch> {
let sanitized_txs = txs
.into_iter()
.map(|tx| SanitizedTransaction::try_create(tx, MessageHash::Compute, None, self))
.map(|tx| {
SanitizedTransaction::try_create(
tx,
MessageHash::Compute,
None,
self,
self.get_reserved_account_keys(),
)
})
.collect::<Result<Vec<_>>>()?;
let tx_account_lock_limit = self.get_transaction_account_lock_limit();
let lock_results = self
Expand Down Expand Up @@ -5901,7 +5909,13 @@ impl Bank {
tx.message.hash()
};

SanitizedTransaction::try_create(tx, message_hash, None, self)
SanitizedTransaction::try_create(
tx,
message_hash,
None,
self,
self.get_reserved_account_keys(),
)
}?;

if verification_mode == TransactionVerificationMode::HashAndVerifyPrecompiles
Expand Down
3 changes: 2 additions & 1 deletion runtime/src/bank/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,8 @@ pub(in crate::bank) fn create_genesis_config(lamports: u64) -> (GenesisConfig, K
}

fn new_sanitized_message(message: Message) -> SanitizedMessage {
SanitizedMessage::try_from_legacy_message(message).unwrap()
SanitizedMessage::try_from_legacy_message(message, &ReservedAccountKeys::empty_key_set())
.unwrap()
}

#[test]
Expand Down
18 changes: 9 additions & 9 deletions runtime/src/bank_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,15 +285,15 @@ impl SyncClient for BankClient {
}

fn get_fee_for_message(&self, message: &Message) -> Result<u64> {
SanitizedMessage::try_from_legacy_message(message.clone())
.ok()
.and_then(|sanitized_message| self.bank.get_fee_for_message(&sanitized_message))
.ok_or_else(|| {
TransportError::IoError(io::Error::new(
io::ErrorKind::Other,
"Unable calculate fee",
))
})
SanitizedMessage::try_from_legacy_message(
message.clone(),
self.bank.get_reserved_account_keys(),
)
.ok()
.and_then(|sanitized_message| self.bank.get_fee_for_message(&sanitized_message))
.ok_or_else(|| {
TransportError::IoError(io::Error::new(io::ErrorKind::Other, "Unable calculate fee"))
})
}
}

Expand Down
13 changes: 8 additions & 5 deletions sdk/program/src/message/legacy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use {
short_vec, system_instruction, system_program, sysvar, wasm_bindgen,
},
lazy_static::lazy_static,
std::{convert::TryFrom, str::FromStr},
std::{collections::HashSet, convert::TryFrom, str::FromStr},
};

lazy_static! {
Expand Down Expand Up @@ -562,9 +562,9 @@ impl Message {
/// locked when loaded for transaction processing in the runtime. This
/// method differs from `is_maybe_writable` because it is aware of the
/// latest reserved accounts which are not allowed to be write locked.
pub fn is_writable(&self, i: usize) -> bool {
pub fn is_writable(&self, i: usize, reserved_account_keys: &HashSet<Pubkey>) -> bool {
(self.is_writable_index(i))
&& !is_builtin_key_or_sysvar(&self.account_keys[i])
&& !reserved_account_keys.contains(&self.account_keys[i])
&& !self.demote_program_id(i)
}

Expand All @@ -583,11 +583,14 @@ impl Message {
}

#[deprecated]
pub fn get_account_keys_by_lock_type(&self) -> (Vec<&Pubkey>, Vec<&Pubkey>) {
pub fn get_account_keys_by_lock_type(
&self,
reserved_account_keys: &HashSet<Pubkey>,
) -> (Vec<&Pubkey>, Vec<&Pubkey>) {
let mut writable_keys = vec![];
let mut readonly_keys = vec![];
for (i, key) in self.account_keys.iter().enumerate() {
if self.is_writable(i) {
if self.is_writable(i, reserved_account_keys) {
writable_keys.push(key);
} else {
readonly_keys.push(key);
Expand Down
Loading

0 comments on commit 7713959

Please sign in to comment.