Skip to content

Commit

Permalink
Fix - SVM account_loader tests (anza-xyz#3448)
Browse files Browse the repository at this point in the history
* Fixes test_load_transaction_accounts_program_account_executable_bypass.

* Fixes test_load_transaction_accounts_data_sizes().

* Removes test_load_transaction_accounts_program_account_not_found_but_loaded().
Lichtso authored Nov 7, 2024
1 parent 54906e7 commit 7403549
Showing 3 changed files with 133 additions and 149 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions svm/Cargo.toml
Original file line number Diff line number Diff line change
@@ -80,6 +80,7 @@ solana-svm = { path = ".", features = ["dev-context-only-utils"] }
solana-svm-conformance = { workspace = true }
solana-transaction-status = { workspace = true }
solana-version = { workspace = true }
solana_rbpf = { workspace = true }
spl-token-2022 = { workspace = true, features = ["no-entrypoint"] }
test-case = { workspace = true }
tokio = { workspace = true, features = ["full"] }
280 changes: 131 additions & 149 deletions svm/src/account_loader.rs
Original file line number Diff line number Diff line change
@@ -576,11 +576,14 @@ mod tests {
solana_compute_budget::{compute_budget::ComputeBudget, compute_budget_limits},
solana_feature_set::FeatureSet,
solana_program_runtime::loaded_programs::{
ProgramCacheEntry, ProgramCacheEntryOwner, ProgramCacheForTxBatch,
ProgramCacheEntry, ProgramCacheEntryOwner, ProgramCacheEntryType,
ProgramCacheForTxBatch,
},
solana_rbpf::program::BuiltinProgram,
solana_sdk::{
account::{Account, AccountSharedData, ReadableAccount, WritableAccount},
bpf_loader, bpf_loader_upgradeable,
bpf_loader,
bpf_loader_upgradeable::{self, UpgradeableLoaderState},
epoch_schedule::EpochSchedule,
hash::Hash,
instruction::{AccountMeta, CompiledInstruction, Instruction},
@@ -601,7 +604,7 @@ mod tests {
transaction::{Result, SanitizedTransaction, Transaction, TransactionError},
transaction_context::{TransactionAccount, TransactionContext},
},
std::{borrow::Cow, cell::RefCell, collections::HashMap, sync::Arc},
std::{borrow::Cow, cell::RefCell, collections::HashMap, fs::File, io::Read, sync::Arc},
};

#[derive(Default)]
@@ -1368,52 +1371,6 @@ mod tests {
);
}

#[test]
fn test_load_transaction_accounts_program_account_not_found_but_loaded() {
let key1 = Keypair::new();
let key2 = Keypair::new();

let message = Message {
account_keys: vec![key1.pubkey(), key2.pubkey()],
header: MessageHeader::default(),
instructions: vec![CompiledInstruction {
program_id_index: 1,
accounts: vec![0],
data: vec![],
}],
recent_blockhash: Hash::default(),
};

let sanitized_message = new_unchecked_sanitized_message(message);
let mut mock_bank = TestCallbacks::default();
let mut account_data = AccountSharedData::default();
account_data.set_lamports(200);
mock_bank.accounts_map.insert(key1.pubkey(), account_data);

let mut error_metrics = TransactionErrorMetrics::default();
let mut loaded_programs = ProgramCacheForTxBatch::default();
loaded_programs.replenish(key2.pubkey(), Arc::new(ProgramCacheEntry::default()));

let sanitized_transaction = SanitizedTransaction::new_for_tests(
sanitized_message,
vec![Signature::new_unique()],
false,
);
let result = load_transaction_accounts(
&mock_bank,
sanitized_transaction.message(),
LoadedTransactionAccount::default(),
&ComputeBudgetLimits::default(),
&mut error_metrics,
None,
&FeatureSet::default(),
&RentCollector::default(),
&loaded_programs,
);

assert_eq!(result.err(), Some(TransactionError::AccountNotFound));
}

#[test]
fn test_load_transaction_accounts_program_account_executable_bypass() {
// currently, the account loader retrieves read-only non-instruction accounts from the program cache
@@ -1449,10 +1406,7 @@ mod tests {
.insert(bpf_loader::id(), loader_data.clone());
mock_bank
.accounts_map
.insert(native_loader::id(), loader_data);

let mut error_metrics = TransactionErrorMetrics::default();
let mut loaded_programs = ProgramCacheForTxBatch::default();
.insert(native_loader::id(), loader_data.clone());

let transaction =
SanitizedTransaction::from_transaction_for_tests(Transaction::new_signed_with_payer(
@@ -1466,32 +1420,17 @@ mod tests {
Hash::default(),
));

let result = load_transaction_accounts(
&mock_bank,
transaction.message(),
LoadedTransactionAccount {
account: account_data.clone(),
..LoadedTransactionAccount::default()
},
&ComputeBudgetLimits::default(),
&mut error_metrics,
None,
&FeatureSet::default(),
&RentCollector::default(),
&loaded_programs,
);

// without cache, program is invalid
assert_eq!(
result.err(),
Some(TransactionError::InvalidProgramForExecution)
);

let mut loaded_programs = ProgramCacheForTxBatch::default();
loaded_programs.replenish(
program_keypair.pubkey(),
Arc::new(ProgramCacheEntry::default()),
Arc::new(ProgramCacheEntry::new_tombstone(
0,
ProgramCacheEntryOwner::LoaderV2,
ProgramCacheEntryType::Closed,
)),
);

let mut error_metrics = TransactionErrorMetrics::default();
let result = load_transaction_accounts(
&mock_bank,
transaction.message(),
@@ -1507,9 +1446,9 @@ mod tests {
&loaded_programs,
);

// with cache, executable flag is bypassed
// Executable flag is bypassed
let mut cached_program = AccountSharedData::default();
cached_program.set_owner(native_loader::id());
cached_program.set_owner(bpf_loader::id());
cached_program.set_executable(true);

assert_eq!(
@@ -1518,6 +1457,7 @@ mod tests {
accounts: vec![
(account_keypair.pubkey(), account_data.clone()),
(program_keypair.pubkey(), cached_program),
(bpf_loader::id(), loader_data),
],
program_indices: vec![vec![1]],
rent: 0,
@@ -2427,6 +2367,42 @@ mod tests {
fn test_load_transaction_accounts_data_sizes() {
let mut mock_bank = TestCallbacks::default();

let program1_keypair = Keypair::new();
let program1 = program1_keypair.pubkey();
let program2 = Pubkey::new_unique();
let programdata2 = Pubkey::new_unique();
use solana_sdk::account_utils::StateMut;

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_executable(true);
program2_account.set_data(vec![0; program2_size as usize]);
program2_account
.set_state(&UpgradeableLoaderState::Program {
programdata_address: programdata2,
})
.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_data(vec![0; program2_size as usize]);
programdata2_account
.set_state(&UpgradeableLoaderState::ProgramData {
slot: 0,
upgrade_authority_address: None,
})
.unwrap();
let mut programdata = programdata2_account.data().to_vec();
let mut file =
File::open("tests/example-programs/hello-solana/hello_solana_program.so").unwrap();
file.read_to_end(&mut programdata).unwrap();
let programdata2_size = programdata.len() as u32;
programdata2_account.set_data(programdata);
mock_bank
.accounts_map
.insert(programdata2, programdata2_account);

let mut next_size = 1;
let mut make_account = |pubkey, owner, executable| {
let size = next_size;
@@ -2447,22 +2423,6 @@ mod tests {
(size as u32, account)
};

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 program1_keypair = Keypair::new();
let program1 = program1_keypair.pubkey();
let (program1_size, _) = make_account(program1, bpf_loader::id(), true);

let program2 = Pubkey::new_unique();
let (program2_size, _) = make_account(program2, bpf_loader_upgradeable::id(), true);

let programdata2 = Pubkey::new_unique();
let (programdata2_size, _) =
make_account(programdata2, bpf_loader_upgradeable::id(), false);

let fee_payer_keypair = Keypair::new();
let fee_payer = fee_payer_keypair.pubkey();
let (fee_payer_size, fee_payer_account) =
@@ -2474,7 +2434,30 @@ mod tests {
let account2 = Pubkey::new_unique();
let (account2_size, _) = make_account(account2, program2, false);

let test_transaction_data_size_with_cache = |transaction, cache, expected_size| {
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 (_program1_size, _) = make_account(program1, bpf_loader::id(), true);

let mut program_cache = ProgramCacheForTxBatch::default();
let program1_entry = ProgramCacheEntry {
account_size: 0,
account_owner: ProgramCacheEntryOwner::LoaderV2,
program: ProgramCacheEntryType::Closed,
..ProgramCacheEntry::default()
};
program_cache.replenish(program1, Arc::new(program1_entry));
let program2_entry = ProgramCacheEntry {
account_size: (program2_size + programdata2_size) as usize,
account_owner: ProgramCacheEntryOwner::LoaderV3,
program: ProgramCacheEntryType::Unloaded(Arc::new(BuiltinProgram::new_mock())),
..ProgramCacheEntry::default()
};
program_cache.replenish(program2, Arc::new(program2_entry));

let test_transaction_data_size = |transaction, expected_size| {
let loaded_transaction_accounts = load_transaction_accounts(
&mock_bank,
&transaction,
@@ -2488,7 +2471,7 @@ mod tests {
None,
&FeatureSet::default(),
&RentCollector::default(),
&cache,
&program_cache,
)
.unwrap();

@@ -2498,7 +2481,7 @@ mod tests {
);
};

let test_data_size_with_cache = |instructions: Vec<_>, cache, expected_size| {
let test_data_size = |instructions: Vec<_>, expected_size| {
let transaction = SanitizedTransaction::from_transaction_for_tests(
Transaction::new_signed_with_payer(
&instructions,
@@ -2508,21 +2491,13 @@ mod tests {
),
);

test_transaction_data_size_with_cache(transaction, cache, expected_size)
test_transaction_data_size(transaction, expected_size)
};

for account_meta in [AccountMeta::new, AccountMeta::new_readonly] {
let test_data_size = |instructions, expected_size| {
test_data_size_with_cache(
instructions,
ProgramCacheForTxBatch::default(),
expected_size,
)
};

// one program plus loader
let ixns = vec![Instruction::new_with_bytes(program1, &[], vec![])];
test_data_size(ixns, program1_size + bpf_loader_size + fee_payer_size);
test_data_size(ixns, bpf_loader_size + fee_payer_size);

// two programs, two loaders, two accounts
let ixns = vec![
@@ -2533,8 +2508,8 @@ mod tests {
ixns,
account1_size
+ account2_size
+ program1_size
+ program2_size
+ programdata2_size
+ bpf_loader_size
+ upgradeable_loader_size
+ fee_payer_size,
@@ -2546,17 +2521,17 @@ mod tests {
&[],
vec![account_meta(account2, false)],
)];
test_data_size(
ixns,
account2_size + program1_size + bpf_loader_size + fee_payer_size,
);
test_data_size(ixns, account2_size + bpf_loader_size + fee_payer_size);

// program and loader counted once
let ixns = vec![
Instruction::new_with_bytes(program1, &[], vec![]),
Instruction::new_with_bytes(program1, &[], vec![]),
Instruction::new_with_bytes(program2, &[], vec![]),
Instruction::new_with_bytes(program2, &[], vec![]),
];
test_data_size(ixns, program1_size + bpf_loader_size + fee_payer_size);
test_data_size(
ixns,
upgradeable_loader_size + program2_size + programdata2_size + fee_payer_size,
);

// native loader not counted if loader
let ixns = vec![Instruction::new_with_bytes(bpf_loader::id(), &[], vec![])];
@@ -2588,11 +2563,24 @@ mod tests {

// loader counted twice if included in instruction
let ixns = vec![Instruction::new_with_bytes(
program1,
program2,
&[],
vec![account_meta(bpf_loader::id(), false)],
vec![account_meta(bpf_loader_upgradeable::id(), false)],
)];
test_data_size(ixns, program1_size + bpf_loader_size * 2 + fee_payer_size);
test_data_size(
ixns,
upgradeable_loader_size * 2 + program2_size + programdata2_size + fee_payer_size,
);

// loader counted twice even if included first
let ixns = vec![
Instruction::new_with_bytes(bpf_loader_upgradeable::id(), &[], vec![]),
Instruction::new_with_bytes(program2, &[], vec![]),
];
test_data_size(
ixns,
upgradeable_loader_size * 2 + program2_size + programdata2_size + fee_payer_size,
);

// cover that case with multiple loaders to be sure
let ixns = vec![
@@ -2614,43 +2602,25 @@ mod tests {
test_data_size(
ixns,
account1_size
+ program1_size
+ program2_size
+ programdata2_size
+ bpf_loader_size * 2
+ upgradeable_loader_size * 2
+ fee_payer_size,
);

// loader counted twice even if included first
let ixns = vec![
Instruction::new_with_bytes(bpf_loader::id(), &[], vec![]),
Instruction::new_with_bytes(program1, &[], vec![]),
];
test_data_size(ixns, program1_size + bpf_loader_size * 2 + fee_payer_size);

// fee-payer counted once
let ixns = vec![Instruction::new_with_bytes(
program1,
&[],
vec![account_meta(fee_payer, false)],
)];
test_data_size(ixns, program1_size + bpf_loader_size + fee_payer_size);

// edge cases involving program cache
let mut program_cache = ProgramCacheForTxBatch::default();

let program2_entry = ProgramCacheEntry {
account_size: (program2_size + programdata2_size) as usize,
account_owner: ProgramCacheEntryOwner::LoaderV3,
..ProgramCacheEntry::default()
};
program_cache.replenish(program2, Arc::new(program2_entry));
test_data_size(ixns, bpf_loader_size + fee_payer_size);

// normal function call uses the combined cache size
let ixns = vec![Instruction::new_with_bytes(program2, &[], vec![])];
test_data_size_with_cache(
test_data_size(
ixns,
program_cache.clone(),
program2_size + programdata2_size + upgradeable_loader_size + fee_payer_size,
);

@@ -2660,22 +2630,28 @@ mod tests {
&[],
vec![account_meta(program2, false)],
)];
test_data_size_with_cache(
test_data_size(
ixns,
program_cache.clone(),
program2_size + upgradeable_loader_size + fee_payer_size,
);

// programdata as instruction account double-counts it
// programdata as readonly instruction account double-counts it
let ixns = vec![Instruction::new_with_bytes(
program2,
&[],
vec![account_meta(programdata2, false)],
)];
test_data_size_with_cache(
let factor = if ixns[0].accounts[0].is_writable {
1
} else {
2
};
test_data_size(
ixns,
program_cache.clone(),
program2_size + programdata2_size * 2 + upgradeable_loader_size + fee_payer_size,
program2_size
+ programdata2_size * factor
+ upgradeable_loader_size
+ fee_payer_size,
);

// both as instruction accounts, for completeness
@@ -2687,17 +2663,23 @@ mod tests {
account_meta(programdata2, false),
],
)];
test_data_size_with_cache(
let factor = if ixns[0].accounts[0].is_writable {
0
} else {
1
};
test_data_size(
ixns,
program_cache.clone(),
program2_size + programdata2_size + upgradeable_loader_size + fee_payer_size,
program2_size
+ programdata2_size * factor
+ upgradeable_loader_size
+ fee_payer_size,
);

// writable program bypasses the cache
let tx = new_unchecked_sanitized_transaction_with_writable_program(program2, fee_payer);
test_transaction_data_size_with_cache(
test_transaction_data_size(
tx,
program_cache.clone(),
program2_size + upgradeable_loader_size + fee_payer_size,
);

0 comments on commit 7403549

Please sign in to comment.