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

v2.1: Fix flaky test_transaction_result_does_not_affect_bankhash (backport of #3916) #4578

Merged
merged 2 commits into from
Jan 23, 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
23 changes: 16 additions & 7 deletions ledger/src/blockstore_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2306,7 +2306,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 All @@ -2333,6 +2334,7 @@ pub mod tests {
pubkey::Pubkey,
rent_debits::RentDebits,
signature::{Keypair, Signer},
signer::SeedDerivable,
system_instruction::SystemError,
system_transaction,
transaction::{Transaction, TransactionError},
Expand All @@ -2349,6 +2351,7 @@ pub mod tests {
vote_transaction,
},
std::{collections::BTreeSet, sync::RwLock},
test_case::test_case,
trees::tr,
};

Expand Down Expand Up @@ -3411,14 +3414,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 @@ -3484,7 +3492,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 @@ -3548,11 +3556,12 @@ 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();

assert_eq!(blockhash_ok, bank.last_blockhash());
assert!(bankhash_ok != bank.hash());
assert_eq!(bankhash_ok == bank.hash(), fee_payer_in_rent_partition);
Copy link

Choose a reason for hiding this comment

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

So the merge conflict needed to bring this check from c5473e4, but keep the rest from v2.1 head

if let Some(bankhash) = bankhash_err {
assert_eq!(bankhash, bank.hash());
}
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
Loading