Skip to content

Commit

Permalink
Fix flaky test_transaction_result_does_not_affect_bankhash (#3916)
Browse files Browse the repository at this point in the history
  • Loading branch information
apfitzge authored Dec 5, 2024
1 parent 5fbdad3 commit c5473e4
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 12 deletions.
25 changes: 17 additions & 8 deletions ledger/src/blockstore_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2353,7 +2353,8 @@ pub mod tests {
crate::{
blockstore_options::{AccessType, BlockstoreOptions},
genesis_utils::{
create_genesis_config, create_genesis_config_with_leader, GenesisConfigInfo,
create_genesis_config, create_genesis_config_with_leader,
create_genesis_config_with_mint_keypair, GenesisConfigInfo,
},
},
assert_matches::assert_matches,
Expand Down Expand Up @@ -2381,6 +2382,7 @@ pub mod tests {
pubkey::Pubkey,
rent_debits::RentDebits,
signature::{Keypair, Signer},
signer::SeedDerivable,
system_instruction::SystemError,
system_transaction,
transaction::{Transaction, TransactionError},
Expand All @@ -2397,6 +2399,7 @@ pub mod tests {
vote_transaction,
},
std::{collections::BTreeSet, sync::RwLock},
test_case::test_case,
trees::tr,
};

Expand Down Expand Up @@ -3459,14 +3462,19 @@ pub mod tests {
}
}

#[test]
fn test_transaction_result_does_not_affect_bankhash() {
#[test_case(true; "rent_collected")]
#[test_case(false; "rent_not_collected")]
fn test_transaction_result_does_not_affect_bankhash(fee_payer_in_rent_partition: bool) {
solana_logger::setup();
let GenesisConfigInfo {
genesis_config,
mint_keypair,
..
} = create_genesis_config(1000);
} = if fee_payer_in_rent_partition {
create_genesis_config(1000)
} else {
create_genesis_config_with_mint_keypair(Keypair::from_seed(&[1u8; 32]).unwrap(), 1000)
};

fn get_instruction_errors() -> Vec<InstructionError> {
vec![
Expand Down Expand Up @@ -3532,7 +3540,7 @@ pub mod tests {
Ok(())
});

let mock_program_id = solana_sdk::pubkey::new_rand();
let mock_program_id = Pubkey::new_unique();

let (bank, _bank_forks) = Bank::new_with_mockup_builtin_for_tests(
&genesis_config,
Expand Down Expand Up @@ -3596,7 +3604,8 @@ pub mod tests {

let entry = next_entry(&bank.last_blockhash(), 1, vec![tx]);
let bank = Arc::new(bank);
let _result = process_entries_for_tests_without_scheduler(&bank, vec![entry]);
let result = process_entries_for_tests_without_scheduler(&bank, vec![entry]);
assert!(result.is_ok()); // No failing transaction error - only instruction errors
bank.freeze();
let bank_details = SlotDetails::new_from_bank(&bank, true).unwrap();

Expand All @@ -3613,8 +3622,8 @@ pub mod tests {
.unwrap()
.last_blockhash
);
// ... but should affect bank hash
assert_ne!(ok_bank_details, bank_details);
// AND should not affect bankhash IF the rent is collected during freeze.
assert_eq!(ok_bank_details == bank_details, fee_payer_in_rent_partition);
// Different types of transaction failure should not affect bank hash
if let Some(prev_bank_details) = &err_bank_details {
assert_eq!(
Expand Down
18 changes: 17 additions & 1 deletion ledger/src/genesis_utils.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,29 @@
pub use solana_runtime::genesis_utils::{
bootstrap_validator_stake_lamports, create_genesis_config_with_leader, GenesisConfigInfo,
};
use {
solana_runtime::genesis_utils::create_genesis_config_with_leader_with_mint_keypair,
solana_sdk::{pubkey::Pubkey, signature::Keypair},
};

// same as genesis_config::create_genesis_config, but with bootstrap_validator staking logic
// for the core crate tests
pub fn create_genesis_config(mint_lamports: u64) -> GenesisConfigInfo {
create_genesis_config_with_leader(
mint_lamports,
&solana_sdk::pubkey::new_rand(),
&Pubkey::new_unique(),
bootstrap_validator_stake_lamports(),
)
}

pub fn create_genesis_config_with_mint_keypair(
mint_keypair: Keypair,
mint_lamports: u64,
) -> GenesisConfigInfo {
create_genesis_config_with_leader_with_mint_keypair(
mint_keypair,
mint_lamports,
&Pubkey::new_unique(),
bootstrap_validator_stake_lamports(),
)
}
32 changes: 29 additions & 3 deletions runtime/src/genesis_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use {
pubkey::Pubkey,
rent::Rent,
signature::{Keypair, Signer},
signer::SeedDerivable,
stake::state::StakeStateV2,
system_program,
},
Expand Down Expand Up @@ -169,15 +170,40 @@ pub fn create_genesis_config_with_leader(
validator_pubkey: &Pubkey,
validator_stake_lamports: u64,
) -> GenesisConfigInfo {
let mint_keypair = Keypair::new();
let voting_keypair = Keypair::new();
// Use deterministic keypair so we don't get confused by randomness in tests
let mint_keypair = Keypair::from_seed(&[
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24,
25, 26, 27, 28, 29, 30, 31,
])
.unwrap();

create_genesis_config_with_leader_with_mint_keypair(
mint_keypair,
mint_lamports,
validator_pubkey,
validator_stake_lamports,
)
}

pub fn create_genesis_config_with_leader_with_mint_keypair(
mint_keypair: Keypair,
mint_lamports: u64,
validator_pubkey: &Pubkey,
validator_stake_lamports: u64,
) -> GenesisConfigInfo {
// Use deterministic keypair so we don't get confused by randomness in tests
let voting_keypair = Keypair::from_seed(&[
32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54,
55, 56, 57, 58, 59, 60, 61, 62, 63,
])
.unwrap();

let genesis_config = create_genesis_config_with_leader_ex(
mint_lamports,
&mint_keypair.pubkey(),
validator_pubkey,
&voting_keypair.pubkey(),
&solana_sdk::pubkey::new_rand(),
&Pubkey::new_unique(),
validator_stake_lamports,
VALIDATOR_LAMPORTS,
FeeRateGovernor::new(0, 0), // most tests can't handle transaction fees
Expand Down

0 comments on commit c5473e4

Please sign in to comment.