Skip to content

Commit

Permalink
Revert - solana-labs#879 and solana-labs#768 (solana-labs#3521)
Browse files Browse the repository at this point in the history 
* Revert "Cleanup - Removes the owner form the result of `filter_executable_program_accounts()` (solana-labs#879)"

This reverts commit 5fe30cb.

* Revert "Refactor - Remove `program_accounts_map` from account_loader (solana-labs#768)"

This reverts commit e7617a1.
  • Loading branch information
@Lichtso
Lichtso authored Nov 8, 2024
1 parent 4edf1ed commit 57bdb8e
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 48 deletions.
77 changes: 58 additions & 19 deletions svm/src/account_loader.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use {
itertools::Itertools,
solana_compute_budget::compute_budget_limits::ComputeBudgetLimits,
solana_feature_set::{self as feature_set, FeatureSet},
solana_program_runtime::loaded_programs::{ProgramCacheEntry, ProgramCacheForTxBatch},
solana_program_runtime::loaded_programs::ProgramCacheForTxBatch,
solana_sdk::{
account::{Account, AccountSharedData, ReadableAccount, WritableAccount},
fee::FeeDetails,
Expand All @@ -30,7 +30,7 @@ use {
solana_svm_rent_collector::svm_rent_collector::SVMRentCollector,
solana_svm_transaction::svm_message::SVMMessage,
solana_system_program::{get_system_account_kind, SystemAccountKind},
std::num::NonZeroU32,
std::{collections::HashMap, num::NonZeroU32},
};

// for the load instructions
Expand Down Expand Up @@ -194,6 +194,7 @@ pub(crate) fn load_accounts<CB: TransactionProcessingCallback>(
account_overrides: Option<&AccountOverrides>,
feature_set: &FeatureSet,
rent_collector: &dyn SVMRentCollector,
program_accounts: &HashMap<Pubkey, (&Pubkey, u64)>,
loaded_programs: &ProgramCacheForTxBatch,
) -> Vec<TransactionLoadResult> {
txs.iter()
Expand All @@ -207,6 +208,7 @@ pub(crate) fn load_accounts<CB: TransactionProcessingCallback>(
account_overrides,
feature_set,
rent_collector,
program_accounts,
loaded_programs,
)
})
Expand All @@ -221,6 +223,7 @@ fn load_transaction<CB: TransactionProcessingCallback>(
account_overrides: Option<&AccountOverrides>,
feature_set: &FeatureSet,
rent_collector: &dyn SVMRentCollector,
program_accounts: &HashMap<Pubkey, (&Pubkey, u64)>,
loaded_programs: &ProgramCacheForTxBatch,
) -> TransactionLoadResult {
match validation_result {
Expand All @@ -235,6 +238,7 @@ fn load_transaction<CB: TransactionProcessingCallback>(
account_overrides,
feature_set,
rent_collector,
program_accounts,
loaded_programs,
);

Expand Down Expand Up @@ -268,6 +272,7 @@ struct LoadedTransactionAccounts {
pub loaded_accounts_data_size: u32,
}

#[allow(clippy::too_many_arguments)]
fn load_transaction_accounts<CB: TransactionProcessingCallback>(
callbacks: &CB,
message: &impl SVMMessage,
Expand All @@ -277,6 +282,7 @@ fn load_transaction_accounts<CB: TransactionProcessingCallback>(
account_overrides: Option<&AccountOverrides>,
feature_set: &FeatureSet,
rent_collector: &dyn SVMRentCollector,
program_accounts: &HashMap<Pubkey, (&Pubkey, u64)>,
loaded_programs: &ProgramCacheForTxBatch,
) -> Result<LoadedTransactionAccounts> {
let mut tx_rent: TransactionRent = 0;
Expand Down Expand Up @@ -331,6 +337,7 @@ fn load_transaction_accounts<CB: TransactionProcessingCallback>(
account_overrides,
feature_set,
rent_collector,
program_accounts,
loaded_programs,
)?;
collect_loaded_account(account_key, (loaded_account, account_found))?;
Expand Down Expand Up @@ -407,6 +414,7 @@ fn load_transaction_accounts<CB: TransactionProcessingCallback>(
})
}

#[allow(clippy::too_many_arguments)]
fn load_transaction_account<CB: TransactionProcessingCallback>(
callbacks: &CB,
message: &impl SVMMessage,
Expand All @@ -416,6 +424,7 @@ fn load_transaction_account<CB: TransactionProcessingCallback>(
account_overrides: Option<&AccountOverrides>,
feature_set: &FeatureSet,
rent_collector: &dyn SVMRentCollector,
program_accounts: &HashMap<Pubkey, (&Pubkey, u64)>,
loaded_programs: &ProgramCacheForTxBatch,
) -> Result<(LoadedTransactionAccount, bool)> {
let mut account_found = true;
Expand Down Expand Up @@ -443,14 +452,11 @@ fn load_transaction_account<CB: TransactionProcessingCallback>(
.then_some(())
.and_then(|_| loaded_programs.find(account_key))
{
callbacks
.get_account_shared_data(account_key)
.ok_or(TransactionError::AccountNotFound)?;
// Optimization to skip loading of accounts which are only used as
// programs in top-level instructions and not passed as instruction accounts.
LoadedTransactionAccount {
loaded_size: program.account_size,
account: account_shared_data_from_program(&program),
account: account_shared_data_from_program(account_key, program_accounts)?,
rent_collected: 0,
}
} else {
Expand Down Expand Up @@ -500,14 +506,20 @@ fn load_transaction_account<CB: TransactionProcessingCallback>(
Ok((loaded_account, account_found))
}

fn account_shared_data_from_program(loaded_program: &ProgramCacheEntry) -> AccountSharedData {
fn account_shared_data_from_program(
key: &Pubkey,
program_accounts: &HashMap<Pubkey, (&Pubkey, u64)>,
) -> Result<AccountSharedData> {
// It's an executable program account. The program is already loaded in the cache.
// So the account data is not needed. Return a dummy AccountSharedData with meta
// information.
let mut program_account = AccountSharedData::default();
program_account.set_owner(loaded_program.account_owner());
let (program_owner, _count) = program_accounts
.get(key)
.ok_or(TransactionError::AccountNotFound)?;
program_account.set_owner(**program_owner);
program_account.set_executable(true);
program_account
Ok(program_account)
}

/// Accumulate loaded account data size into `accumulated_accounts_data_size`.
Expand Down Expand Up @@ -674,6 +686,7 @@ mod tests {
None,
feature_set,
rent_collector,
&HashMap::new(),
&ProgramCacheForTxBatch::default(),
)
}
Expand Down Expand Up @@ -990,6 +1003,7 @@ mod tests {
account_overrides,
&FeatureSet::all_enabled(),
&RentCollector::default(),
&HashMap::new(),
&ProgramCacheForTxBatch::default(),
)
}
Expand Down Expand Up @@ -1285,6 +1299,7 @@ mod tests {
None,
&FeatureSet::default(),
&RentCollector::default(),
&HashMap::new(),
&loaded_programs,
);

Expand Down Expand Up @@ -1350,6 +1365,7 @@ mod tests {
None,
&FeatureSet::default(),
&RentCollector::default(),
&HashMap::new(),
&loaded_programs,
);

Expand Down Expand Up @@ -1383,6 +1399,7 @@ mod tests {
let mut mock_bank = TestCallbacks::default();
let account_keypair = Keypair::new();
let program_keypair = Keypair::new();
let loader_v2 = bpf_loader::id();

let mut account_data = AccountSharedData::default();
account_data.set_lamports(200);
Expand All @@ -1392,7 +1409,7 @@ mod tests {

let mut program_data = AccountSharedData::default();
program_data.set_lamports(200);
program_data.set_owner(bpf_loader::id());
program_data.set_owner(loader_v2);
mock_bank
.accounts_map
.insert(program_keypair.pubkey(), program_data);
Expand All @@ -1403,7 +1420,7 @@ mod tests {
loader_data.set_owner(native_loader::id());
mock_bank
.accounts_map
.insert(bpf_loader::id(), loader_data.clone());
.insert(loader_v2, loader_data.clone());
mock_bank
.accounts_map
.insert(native_loader::id(), loader_data.clone());
Expand All @@ -1420,6 +1437,8 @@ mod tests {
Hash::default(),
));

let mut program_accounts = HashMap::new();
program_accounts.insert(program_keypair.pubkey(), (&loader_v2, 0));
let mut loaded_programs = ProgramCacheForTxBatch::default();
loaded_programs.replenish(
program_keypair.pubkey(),
Expand All @@ -1443,12 +1462,13 @@ mod tests {
None,
&FeatureSet::default(),
&RentCollector::default(),
&program_accounts,
&loaded_programs,
);

// Executable flag is bypassed
let mut cached_program = AccountSharedData::default();
cached_program.set_owner(bpf_loader::id());
cached_program.set_owner(loader_v2);
cached_program.set_executable(true);

assert_eq!(
Expand All @@ -1457,7 +1477,7 @@ mod tests {
accounts: vec![
(account_keypair.pubkey(), account_data.clone()),
(program_keypair.pubkey(), cached_program),
(bpf_loader::id(), loader_data),
(loader_v2, loader_data),
],
program_indices: vec![vec![1]],
rent: 0,
Expand Down Expand Up @@ -1490,6 +1510,7 @@ mod tests {
None,
&FeatureSet::default(),
&RentCollector::default(),
&program_accounts,
&loaded_programs,
);

Expand All @@ -1516,6 +1537,7 @@ mod tests {
None,
&FeatureSet::default(),
&RentCollector::default(),
&program_accounts,
&loaded_programs,
);

Expand Down Expand Up @@ -1565,6 +1587,7 @@ mod tests {
None,
&FeatureSet::default(),
&RentCollector::default(),
&HashMap::new(),
&loaded_programs,
);

Expand Down Expand Up @@ -1610,6 +1633,7 @@ mod tests {
None,
&FeatureSet::default(),
&RentCollector::default(),
&HashMap::new(),
&loaded_programs,
);

Expand Down Expand Up @@ -1667,6 +1691,7 @@ mod tests {
None,
&FeatureSet::default(),
&RentCollector::default(),
&HashMap::new(),
&loaded_programs,
);

Expand Down Expand Up @@ -1730,6 +1755,7 @@ mod tests {
None,
&FeatureSet::default(),
&RentCollector::default(),
&HashMap::new(),
&loaded_programs,
);

Expand Down Expand Up @@ -1784,6 +1810,7 @@ mod tests {
None,
&FeatureSet::default(),
&RentCollector::default(),
&HashMap::new(),
&loaded_programs,
);

Expand Down Expand Up @@ -1848,6 +1875,7 @@ mod tests {
None,
&FeatureSet::default(),
&RentCollector::default(),
&HashMap::new(),
&loaded_programs,
);

Expand Down Expand Up @@ -1936,6 +1964,7 @@ mod tests {
None,
&FeatureSet::default(),
&RentCollector::default(),
&HashMap::new(),
&loaded_programs,
);

Expand Down Expand Up @@ -2001,6 +2030,7 @@ mod tests {
None,
&FeatureSet::default(),
&RentCollector::default(),
&HashMap::new(),
&ProgramCacheForTxBatch::default(),
);

Expand Down Expand Up @@ -2097,6 +2127,7 @@ mod tests {
None,
&FeatureSet::default(),
&RentCollector::default(),
&HashMap::new(),
&loaded_programs,
);

Expand Down Expand Up @@ -2169,6 +2200,7 @@ mod tests {
None,
&feature_set,
&rent_collector,
&HashMap::new(),
&ProgramCacheForTxBatch::default(),
);

Expand All @@ -2190,6 +2222,7 @@ mod tests {
None,
&feature_set,
&rent_collector,
&HashMap::new(),
&ProgramCacheForTxBatch::default(),
);

Expand Down Expand Up @@ -2340,6 +2373,7 @@ mod tests {
None,
&FeatureSet::default(),
&RentCollector::default(),
&HashMap::new(),
&ProgramCacheForTxBatch::default(),
);

Expand Down Expand Up @@ -2367,6 +2401,8 @@ mod tests {
fn test_load_transaction_accounts_data_sizes() {
let mut mock_bank = TestCallbacks::default();

let loader_v2 = bpf_loader::id();
let loader_v3 = bpf_loader_upgradeable::id();
let program1_keypair = Keypair::new();
let program1 = program1_keypair.pubkey();
let program2 = Pubkey::new_unique();
Expand All @@ -2375,7 +2411,7 @@ mod tests {

let program2_size = std::mem::size_of::<UpgradeableLoaderState>() as u32;
let mut program2_account = AccountSharedData::default();
program2_account.set_owner(bpf_loader_upgradeable::id());
program2_account.set_owner(loader_v3);
program2_account.set_executable(true);
program2_account.set_data(vec![0; program2_size as usize]);
program2_account
Expand All @@ -2385,7 +2421,7 @@ mod tests {
.unwrap();
mock_bank.accounts_map.insert(program2, program2_account);
let mut programdata2_account = AccountSharedData::default();
programdata2_account.set_owner(bpf_loader_upgradeable::id());
programdata2_account.set_owner(loader_v3);
programdata2_account.set_data(vec![0; program2_size as usize]);
programdata2_account
.set_state(&UpgradeableLoaderState::ProgramData {
Expand Down Expand Up @@ -2435,12 +2471,14 @@ mod tests {
let (account2_size, _) = make_account(account2, program2, false);

let (native_loader_size, _) = make_account(native_loader::id(), native_loader::id(), true);
let (bpf_loader_size, _) = make_account(bpf_loader::id(), native_loader::id(), true);
let (upgradeable_loader_size, _) =
make_account(bpf_loader_upgradeable::id(), native_loader::id(), true);
let (bpf_loader_size, _) = make_account(loader_v2, native_loader::id(), true);
let (upgradeable_loader_size, _) = make_account(loader_v3, native_loader::id(), true);

let (_program1_size, _) = make_account(program1, bpf_loader::id(), true);
let (_program1_size, _) = make_account(program1, loader_v2, true);

let mut program_accounts = HashMap::new();
program_accounts.insert(program1, (&loader_v2, 0));
program_accounts.insert(program2, (&loader_v3, 0));
let mut program_cache = ProgramCacheForTxBatch::default();
let program1_entry = ProgramCacheEntry {
account_size: 0,
Expand Down Expand Up @@ -2471,6 +2509,7 @@ mod tests {
None,
&FeatureSet::default(),
&RentCollector::default(),
&program_accounts,
&program_cache,
)
.unwrap();
Expand Down
Loading

0 comments on commit 57bdb8e

Please sign in to comment.