Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce unnecessary public visibility of some SVM structs and APIs #4308

Merged
merged 3 commits into from
Jan 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 26 additions & 20 deletions svm/src/account_loader.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#[cfg(feature = "dev-context-only-utils")]
use qualifier_attr::field_qualifiers;
use {
crate::{
account_overrides::AccountOverrides,
Expand Down Expand Up @@ -42,10 +44,10 @@ use {
pub(crate) type TransactionRent = u64;
pub(crate) type TransactionProgramIndices = Vec<Vec<IndexOfAccount>>;
pub type TransactionCheckResult = Result<CheckedTransactionDetails>;
pub type TransactionValidationResult = Result<ValidatedTransactionDetails>;
type TransactionValidationResult = Result<ValidatedTransactionDetails>;

#[derive(PartialEq, Eq, Debug)]
pub enum TransactionLoadResult {
pub(crate) enum TransactionLoadResult {
/// All transaction accounts were loaded successfully
Loaded(LoadedTransaction),
/// Some transaction accounts needed for execution were unable to be loaded
Expand All @@ -66,29 +68,33 @@ pub struct CheckedTransactionDetails {

#[derive(PartialEq, Eq, Debug, Clone)]
#[cfg_attr(feature = "dev-context-only-utils", derive(Default))]
pub struct ValidatedTransactionDetails {
pub rollback_accounts: RollbackAccounts,
pub compute_budget_limits: ComputeBudgetLimits,
pub fee_details: FeeDetails,
pub loaded_fee_payer_account: LoadedTransactionAccount,
pub(crate) struct ValidatedTransactionDetails {
pub(crate) rollback_accounts: RollbackAccounts,
pub(crate) compute_budget_limits: ComputeBudgetLimits,
pub(crate) fee_details: FeeDetails,
pub(crate) loaded_fee_payer_account: LoadedTransactionAccount,
}

#[derive(PartialEq, Eq, Debug, Clone)]
#[cfg_attr(feature = "dev-context-only-utils", derive(Default))]
pub struct LoadedTransactionAccount {
pub(crate) struct LoadedTransactionAccount {
pub(crate) account: AccountSharedData,
pub(crate) loaded_size: usize,
pub(crate) rent_collected: u64,
}

#[derive(PartialEq, Eq, Debug, Clone)]
#[cfg_attr(feature = "dev-context-only-utils", derive(Default))]
#[cfg_attr(
feature = "dev-context-only-utils",
field_qualifiers(program_indices(pub), compute_budget_limits(pub))
)]
pub struct LoadedTransaction {
pub accounts: Vec<TransactionAccount>,
pub program_indices: TransactionProgramIndices,
pub(crate) program_indices: TransactionProgramIndices,
pub fee_details: FeeDetails,
pub rollback_accounts: RollbackAccounts,
pub compute_budget_limits: ComputeBudgetLimits,
pub(crate) compute_budget_limits: ComputeBudgetLimits,
pub rent: TransactionRent,
pub rent_debits: RentDebits,
pub loaded_accounts_data_size: u32,
Expand All @@ -110,7 +116,7 @@ pub(crate) struct AccountLoader<'a, CB: TransactionProcessingCallback> {
pub(crate) feature_set: Arc<FeatureSet>,
}
impl<'a, CB: TransactionProcessingCallback> AccountLoader<'a, CB> {
pub fn new_with_account_cache_capacity(
pub(crate) fn new_with_account_cache_capacity(
account_overrides: Option<&'a AccountOverrides>,
program_cache: ProgramCacheForTxBatch,
program_accounts: HashMap<Pubkey, (&'a Pubkey, u64)>,
Expand All @@ -137,7 +143,7 @@ impl<'a, CB: TransactionProcessingCallback> AccountLoader<'a, CB> {
}
}

pub fn load_account(
pub(crate) fn load_account(
&mut self,
account_key: &Pubkey,
usage_pattern: AccountUsagePattern,
Expand Down Expand Up @@ -196,7 +202,7 @@ impl<'a, CB: TransactionProcessingCallback> AccountLoader<'a, CB> {
})
}

pub fn update_accounts_for_executed_tx(
pub(crate) fn update_accounts_for_executed_tx(
&mut self,
message: &impl SVMMessage,
executed_transaction: &ExecutedTransaction,
Expand All @@ -217,7 +223,7 @@ impl<'a, CB: TransactionProcessingCallback> AccountLoader<'a, CB> {
}
}

pub fn update_accounts_for_failed_tx(
pub(crate) fn update_accounts_for_failed_tx(
&mut self,
message: &impl SVMMessage,
rollback_accounts: &RollbackAccounts,
Expand Down Expand Up @@ -274,7 +280,7 @@ pub(crate) enum AccountUsagePattern {
ReadOnlyInvisible,
}
impl AccountUsagePattern {
pub fn new(message: &impl SVMMessage, account_index: usize) -> Self {
fn new(message: &impl SVMMessage, account_index: usize) -> Self {
let is_writable = message.is_writable(account_index);
let is_instruction_account = message.is_instruction_account(account_index);

Expand Down Expand Up @@ -414,11 +420,11 @@ pub(crate) fn load_transaction<CB: TransactionProcessingCallback>(

#[derive(PartialEq, Eq, Debug, Clone)]
struct LoadedTransactionAccounts {
pub accounts: Vec<TransactionAccount>,
pub program_indices: TransactionProgramIndices,
pub rent: TransactionRent,
pub rent_debits: RentDebits,
pub loaded_accounts_data_size: u32,
pub(crate) accounts: Vec<TransactionAccount>,
pub(crate) program_indices: TransactionProgramIndices,
pub(crate) rent: TransactionRent,
pub(crate) rent_debits: RentDebits,
pub(crate) loaded_accounts_data_size: u32,
}

fn load_transaction_accounts<CB: TransactionProcessingCallback>(
Expand Down
2 changes: 1 addition & 1 deletion svm/src/account_overrides.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ impl AccountOverrides {
}

/// Gets the account if it's found in the list of overrides
pub fn get(&self, pubkey: &Pubkey) -> Option<&AccountSharedData> {
pub(crate) fn get(&self, pubkey: &Pubkey) -> Option<&AccountSharedData> {
self.accounts.get(pubkey)
}
}
Expand Down
4 changes: 2 additions & 2 deletions svm/src/message_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use {
};

#[derive(Debug, Default, Clone, serde_derive::Deserialize, serde_derive::Serialize)]
pub struct MessageProcessor {}
pub(crate) struct MessageProcessor {}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe some people use MessageProcessor::process_message in their libraries, such as LiteSVM. Any objection to keeping this one public, and just keeping it within the "not guaranteed under SemVer" band?

cc @kevinheavey

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like MessageProcessor to be pub yes. But I guess I was already planning on copy-pasting it since litesvm doesn't use anything else from solana-svm


#[cfg(feature = "frozen-abi")]
impl ::solana_frozen_abi::abi_example::AbiExample for MessageProcessor {
Expand All @@ -28,7 +28,7 @@ impl MessageProcessor {
/// For each instruction it calls the program entrypoint method and verifies that the result of
/// the call does not violate the bank's accounting rules.
/// The accounts are committed back to the bank only if every instruction succeeds.
pub fn process_message(
pub(crate) fn process_message(
message: &impl SVMMessage,
program_indices: &[Vec<IndexOfAccount>],
invoke_context: &mut InvokeContext,
Expand Down
14 changes: 10 additions & 4 deletions svm/src/nonce_info.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
#[cfg(feature = "dev-context-only-utils")]
use {
solana_account::{state_traits::StateMut, AccountSharedData},
qualifier_attr::qualifiers,
solana_account::state_traits::StateMut,
solana_nonce::{
state::{DurableNonce, State as NonceState},
versions::Versions as NonceVersions,
},
solana_pubkey::Pubkey,
thiserror::Error,
};
use {solana_account::AccountSharedData, solana_pubkey::Pubkey};

/// Holds limited nonce info available during transaction checks
#[derive(Clone, Debug, Default, PartialEq, Eq)]
Expand All @@ -16,7 +18,9 @@ pub struct NonceInfo {
}

#[derive(Error, Debug, PartialEq)]
pub enum AdvanceNonceError {
#[cfg(feature = "dev-context-only-utils")]
#[cfg_attr(feature = "dev-context-only-utils", qualifiers(pub))]
enum AdvanceNonceError {
#[error("Invalid account")]
Invalid,
#[error("Uninitialized nonce")]
Expand All @@ -31,7 +35,9 @@ impl NonceInfo {
// Advance the stored blockhash to prevent fee theft by someone
// replaying nonce transactions that have failed with an
// `InstructionError`.
pub fn try_advance_nonce(
#[cfg(feature = "dev-context-only-utils")]
#[cfg_attr(feature = "dev-context-only-utils", qualifiers(pub))]
fn try_advance_nonce(
&mut self,
durable_nonce: DurableNonce,
lamports_per_signature: u64,
Expand Down
4 changes: 2 additions & 2 deletions svm/src/program_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,8 @@ mod tests {
}

#[derive(Default, Clone)]
pub struct MockBankCallback {
pub account_shared_data: RefCell<HashMap<Pubkey, AccountSharedData>>,
pub(crate) struct MockBankCallback {
pub(crate) account_shared_data: RefCell<HashMap<Pubkey, AccountSharedData>>,
}

impl TransactionProcessingCallback for MockBankCallback {
Expand Down
2 changes: 1 addition & 1 deletion svm/src/rollback_accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ impl Default for RollbackAccounts {
}

impl RollbackAccounts {
pub fn new(
pub(crate) fn new(
nonce: Option<NonceInfo>,
fee_payer_address: Pubkey,
mut fee_payer_account: AccountSharedData,
Expand Down
9 changes: 5 additions & 4 deletions svm/src/transaction_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1192,7 +1192,8 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
}

#[cfg(feature = "dev-context-only-utils")]
pub fn writable_sysvar_cache(&self) -> &RwLock<SysvarCache> {
#[cfg_attr(feature = "dev-context-only-utils", qualifiers(pub))]
fn writable_sysvar_cache(&self) -> &RwLock<SysvarCache> {
&self.sysvar_cache
}
}
Expand Down Expand Up @@ -1250,10 +1251,10 @@ mod tests {
}

#[derive(Default, Clone)]
pub struct MockBankCallback {
pub account_shared_data: Arc<RwLock<HashMap<Pubkey, AccountSharedData>>>,
struct MockBankCallback {
account_shared_data: Arc<RwLock<HashMap<Pubkey, AccountSharedData>>>,
#[allow(clippy::type_complexity)]
pub inspected_accounts:
inspected_accounts:
Arc<RwLock<HashMap<Pubkey, Vec<(Option<AccountSharedData>, /* is_writable */ bool)>>>>,
}

Expand Down
Loading