Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Define PohRecorder set_bank related test helper methods #33626

Merged
merged 1 commit into from
Oct 11, 2023
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
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.

5 changes: 4 additions & 1 deletion banking-bench/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,15 @@ solana-ledger = { workspace = true }
solana-logger = { workspace = true }
solana-measure = { workspace = true }
solana-perf = { workspace = true }
solana-poh = { workspace = true }
solana-poh = { workspace = true, features = ["dev-context-only-utils"] }
solana-runtime = { workspace = true }
solana-sdk = { workspace = true }
solana-streamer = { workspace = true }
solana-tpu-client = { workspace = true }
solana-version = { workspace = true }

[features]
dev-context-only-utils = []

[package.metadata.docs.rs]
targets = ["x86_64-unknown-linux-gnu"]
5 changes: 4 additions & 1 deletion banking-bench/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,10 @@ fn main() {
);

assert!(poh_recorder.read().unwrap().bank().is_none());
poh_recorder.write().unwrap().set_bank(bank.clone(), false);
poh_recorder
.write()
.unwrap()
.set_bank_for_test(bank.clone());
assert!(poh_recorder.read().unwrap().bank().is_some());
debug!(
"new_bank_time: {}us insert_time: {}us poh_time: {}us",
Expand Down
1 change: 1 addition & 0 deletions core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ serial_test = { workspace = true }
# See order-crates-for-publishing.py for using this unusual `path = "."`
solana-core = { path = ".", features = ["dev-context-only-utils"] }
solana-logger = { workspace = true }
solana-poh = { workspace = true, features = ["dev-context-only-utils"] }
solana-program-runtime = { workspace = true }
solana-runtime = { workspace = true, features = ["dev-context-only-utils"] }
solana-sdk = { workspace = true, features = ["dev-context-only-utils"] }
Expand Down
5 changes: 4 additions & 1 deletion core/src/banking_stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1071,7 +1071,10 @@ mod tests {

let poh_simulator = simulate_poh(record_receiver, &poh_recorder);

poh_recorder.write().unwrap().set_bank(bank.clone(), false);
poh_recorder
.write()
.unwrap()
.set_bank_for_test(bank.clone());
let pubkey = solana_sdk::pubkey::new_rand();
let keypair2 = Keypair::new();
let pubkey2 = solana_sdk::pubkey::new_rand();
Expand Down
15 changes: 12 additions & 3 deletions core/src/banking_stage/consume_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,10 @@ mod tests {
..
} = &test_frame;
let worker_thread = std::thread::spawn(move || worker.run());
poh_recorder.write().unwrap().set_bank(bank.clone(), false);
poh_recorder
.write()
.unwrap()
.set_bank_for_test(bank.clone());

let pubkey1 = Pubkey::new_unique();

Expand Down Expand Up @@ -325,7 +328,10 @@ mod tests {
..
} = &test_frame;
let worker_thread = std::thread::spawn(move || worker.run());
poh_recorder.write().unwrap().set_bank(bank.clone(), false);
poh_recorder
.write()
.unwrap()
.set_bank_for_test(bank.clone());

let pubkey1 = Pubkey::new_unique();
let pubkey2 = Pubkey::new_unique();
Expand Down Expand Up @@ -370,7 +376,10 @@ mod tests {
..
} = &test_frame;
let worker_thread = std::thread::spawn(move || worker.run());
poh_recorder.write().unwrap().set_bank(bank.clone(), false);
poh_recorder
.write()
.unwrap()
.set_bank_for_test(bank.clone());

let pubkey1 = Pubkey::new_unique();
let pubkey2 = Pubkey::new_unique();
Expand Down
46 changes: 35 additions & 11 deletions core/src/banking_stage/consumer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,10 @@ mod tests {
let recorder = poh_recorder.new_recorder();
let poh_recorder = Arc::new(RwLock::new(poh_recorder));

poh_recorder.write().unwrap().set_bank(bank.clone(), false);
poh_recorder
.write()
.unwrap()
.set_bank_for_test(bank.clone());

let poh_simulator = simulate_poh(record_receiver, &poh_recorder);

Expand Down Expand Up @@ -966,7 +969,10 @@ mod tests {

let poh_simulator = simulate_poh(record_receiver, &poh_recorder);

poh_recorder.write().unwrap().set_bank(bank.clone(), false);
poh_recorder
.write()
.unwrap()
.set_bank_for_test(bank.clone());
let (replay_vote_sender, _replay_vote_receiver) = unbounded();
let committer = Committer::new(
None,
Expand Down Expand Up @@ -1093,7 +1099,10 @@ mod tests {

let poh_simulator = simulate_poh(record_receiver, &poh_recorder);

poh_recorder.write().unwrap().set_bank(bank.clone(), false);
poh_recorder
.write()
.unwrap()
.set_bank_for_test(bank.clone());
let (replay_vote_sender, _replay_vote_receiver) = unbounded();
let committer = Committer::new(
None,
Expand Down Expand Up @@ -1179,7 +1188,10 @@ mod tests {

let poh_simulator = simulate_poh(record_receiver, &poh_recorder);

poh_recorder.write().unwrap().set_bank(bank.clone(), false);
poh_recorder
.write()
.unwrap()
.set_bank_for_test(bank.clone());
let (replay_vote_sender, _replay_vote_receiver) = unbounded();
let committer = Committer::new(
None,
Expand Down Expand Up @@ -1328,7 +1340,10 @@ mod tests {
let recorder = poh_recorder.new_recorder();
let poh_recorder = Arc::new(RwLock::new(poh_recorder));

poh_recorder.write().unwrap().set_bank(bank.clone(), false);
poh_recorder
.write()
.unwrap()
.set_bank_for_test(bank.clone());

let poh_simulator = simulate_poh(record_receiver, &poh_recorder);

Expand Down Expand Up @@ -1628,7 +1643,10 @@ mod tests {

let poh_simulator = simulate_poh(record_receiver, &poh_recorder);

poh_recorder.write().unwrap().set_bank(bank.clone(), false);
poh_recorder
.write()
.unwrap()
.set_bank_for_test(bank.clone());

let shreds = entries_to_test_shreds(
&entries,
Expand Down Expand Up @@ -1765,7 +1783,10 @@ mod tests {

let poh_simulator = simulate_poh(record_receiver, &poh_recorder);

poh_recorder.write().unwrap().set_bank(bank.clone(), false);
poh_recorder
.write()
.unwrap()
.set_bank_for_test(bank.clone());

let shreds = entries_to_test_shreds(
&entries,
Expand Down Expand Up @@ -1864,7 +1885,7 @@ mod tests {
assert_eq!(buffered_packet_batches.len(), num_conflicting_transactions);
// When the working bank in poh_recorder is Some, all packets should be processed.
// Multi-Iterator will process them 1-by-1 if all txs are conflicting.
poh_recorder.write().unwrap().set_bank(bank, false);
poh_recorder.write().unwrap().set_bank_for_test(bank);
let bank_start = poh_recorder.read().unwrap().bank_start().unwrap();
let banking_stage_stats = BankingStageStats::default();
consumer.consume_buffered_packets(
Expand Down Expand Up @@ -1942,7 +1963,7 @@ mod tests {
assert_eq!(buffered_packet_batches.len(), num_conflicting_transactions);
// When the working bank in poh_recorder is Some, all packets should be processed.
// Multi-Iterator will process them 1-by-1 if all txs are conflicting.
poh_recorder.write().unwrap().set_bank(bank, false);
poh_recorder.write().unwrap().set_bank_for_test(bank);
let bank_start = poh_recorder.read().unwrap().bank_start().unwrap();
consumer.consume_buffered_packets(
&bank_start,
Expand Down Expand Up @@ -1995,7 +2016,10 @@ mod tests {
// When the working bank in poh_recorder is Some, all packets should be processed
// except except for retryable errors. Manually take the lock of a transaction to
// simulate another thread processing a transaction with that lock.
poh_recorder.write().unwrap().set_bank(bank.clone(), false);
poh_recorder
.write()
.unwrap()
.set_bank_for_test(bank.clone());
let bank_start = poh_recorder.read().unwrap().bank_start().unwrap();

let lock_account = transactions[0].message.account_keys[1];
Expand Down Expand Up @@ -2116,7 +2140,7 @@ mod tests {
assert_eq!(buffered_packet_batches.len(), num_conflicting_transactions);
// When the working bank in poh_recorder is Some, all packets should be processed.
// Multi-Iterator will process them 1-by-1 if all txs are conflicting.
poh_recorder.write().unwrap().set_bank(bank, false);
poh_recorder.write().unwrap().set_bank_for_test(bank);
let bank_start = poh_recorder.read().unwrap().bank_start().unwrap();
let banking_stage_stats = BankingStageStats::default();
consumer.consume_buffered_packets(
Expand Down
5 changes: 4 additions & 1 deletion core/src/banking_stage/decision_maker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,10 @@ mod tests {

// Currently Leader - Consume
{
poh_recorder.write().unwrap().set_bank(bank.clone(), false);
poh_recorder
.write()
.unwrap()
.set_bank_for_test(bank.clone());
let decision = decision_maker.make_consume_or_forward_decision();
assert_matches!(decision, BufferedPacketsDecision::Consume(_));
}
Expand Down
4 changes: 4 additions & 0 deletions poh/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ bincode = { workspace = true }
rand = { workspace = true }
solana-logger = { workspace = true }
solana-perf = { workspace = true }
solana-poh = { path = ".", features = ["dev-context-only-utils"] }

[features]
dev-context-only-utils = []

[lib]
crate-type = ["lib"]
Expand Down
38 changes: 24 additions & 14 deletions poh/src/poh_recorder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,16 @@ impl PohRecorder {
let _ = self.flush_cache(false);
}

#[cfg(feature = "dev-context-only-utils")]
pub fn set_bank_for_test(&mut self, bank: Arc<Bank>) {
self.set_bank(bank, false)
Copy link
Contributor Author

@ryoqun ryoqun Oct 10, 2023

Choose a reason for hiding this comment

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

i eliminated these falses at all call-sites as a bonus.

}

#[cfg(test)]
pub fn set_bank_with_transaction_index_for_test(&mut self, bank: Arc<Bank>) {
Copy link
Contributor Author

@ryoqun ryoqun Oct 10, 2023

Choose a reason for hiding this comment

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

fyi, this is only used once.... another justification for hiding the track_transaction_indexes bool arg at this wrapper...

self.set_bank(bank, true)
}

// Flush cache will delay flushing the cache for a bank until it past the WorkingBank::min_tick_height
// On a record flush will flush the cache at the WorkingBank::min_tick_height, since a record
// occurs after the min_tick_height was generated
Expand Down Expand Up @@ -1219,7 +1229,7 @@ mod tests {
Arc::new(AtomicBool::default()),
);

poh_recorder.set_bank(bank, false);
poh_recorder.set_bank_for_test(bank);
assert!(poh_recorder.working_bank.is_some());
poh_recorder.clear_bank();
assert!(poh_recorder.working_bank.is_none());
Expand Down Expand Up @@ -1253,7 +1263,7 @@ mod tests {
let bank1 = Arc::new(Bank::new_from_parent(bank0, &Pubkey::default(), 1));

// Set a working bank
poh_recorder.set_bank(bank1.clone(), false);
poh_recorder.set_bank_for_test(bank1.clone());

// Tick until poh_recorder.tick_height == working bank's min_tick_height
let num_new_ticks = bank1.tick_height() - poh_recorder.tick_height();
Expand Down Expand Up @@ -1322,7 +1332,7 @@ mod tests {
);
assert_eq!(poh_recorder.tick_height, bank.max_tick_height() + 1);

poh_recorder.set_bank(bank.clone(), false);
poh_recorder.set_bank_for_test(bank.clone());
poh_recorder.tick();

assert_eq!(poh_recorder.tick_height, bank.max_tick_height() + 2);
Expand Down Expand Up @@ -1363,7 +1373,7 @@ mod tests {

bank0.fill_bank_with_ticks_for_tests();
let bank1 = Arc::new(Bank::new_from_parent(bank0, &Pubkey::default(), 1));
poh_recorder.set_bank(bank1.clone(), false);
poh_recorder.set_bank_for_test(bank1.clone());
// Let poh_recorder tick up to bank1.tick_height() - 1
for _ in 0..bank1.tick_height() - 1 {
poh_recorder.tick()
Expand Down Expand Up @@ -1404,7 +1414,7 @@ mod tests {
Arc::new(AtomicBool::default()),
);

poh_recorder.set_bank(bank.clone(), false);
poh_recorder.set_bank_for_test(bank.clone());
let tx = test_tx();
let h1 = hash(b"hello world!");

Expand Down Expand Up @@ -1448,7 +1458,7 @@ mod tests {

bank0.fill_bank_with_ticks_for_tests();
let bank1 = Arc::new(Bank::new_from_parent(bank0, &Pubkey::default(), 1));
poh_recorder.set_bank(bank1.clone(), false);
poh_recorder.set_bank_for_test(bank1.clone());

// Record up to exactly min tick height
let min_tick_height = poh_recorder.working_bank.as_ref().unwrap().min_tick_height;
Expand Down Expand Up @@ -1502,7 +1512,7 @@ mod tests {
Arc::new(AtomicBool::default()),
);

poh_recorder.set_bank(bank.clone(), false);
poh_recorder.set_bank_for_test(bank.clone());
let num_ticks_to_max = bank.max_tick_height() - poh_recorder.tick_height;
for _ in 0..num_ticks_to_max {
poh_recorder.tick();
Expand Down Expand Up @@ -1542,7 +1552,7 @@ mod tests {
Arc::new(AtomicBool::default()),
);

poh_recorder.set_bank(bank.clone(), true);
poh_recorder.set_bank_with_transaction_index_for_test(bank.clone());
poh_recorder.tick();
assert_eq!(
poh_recorder
Expand Down Expand Up @@ -1616,7 +1626,7 @@ mod tests {

bank0.fill_bank_with_ticks_for_tests();
let bank1 = Arc::new(Bank::new_from_parent(bank0, &Pubkey::default(), 1));
poh_recorder.set_bank(bank1, false);
poh_recorder.set_bank_for_test(bank1);

// Check we can make two ticks without hitting min_tick_height
let remaining_ticks_to_min =
Expand Down Expand Up @@ -1764,7 +1774,7 @@ mod tests {
Arc::new(AtomicBool::default()),
);

poh_recorder.set_bank(bank.clone(), false);
poh_recorder.set_bank_for_test(bank.clone());
assert_eq!(bank.slot(), 0);
poh_recorder.reset(bank, Some((4, 4)));
assert!(poh_recorder.working_bank.is_none());
Expand Down Expand Up @@ -1796,7 +1806,7 @@ mod tests {
None,
Arc::new(AtomicBool::default()),
);
poh_recorder.set_bank(bank, false);
poh_recorder.set_bank_for_test(bank);
poh_recorder.clear_bank();
assert!(receiver.try_recv().is_ok());
}
Expand Down Expand Up @@ -1831,7 +1841,7 @@ mod tests {
Arc::new(AtomicBool::default()),
);

poh_recorder.set_bank(bank.clone(), false);
poh_recorder.set_bank_for_test(bank.clone());

// Simulate ticking much further than working_bank.max_tick_height
let max_tick_height = poh_recorder.working_bank.as_ref().unwrap().max_tick_height;
Expand Down Expand Up @@ -2126,7 +2136,7 @@ mod tests {
// Move the bank up a slot (so that max_tick_height > slot 0's tick_height)
let bank = Arc::new(Bank::new_from_parent(bank, &Pubkey::default(), 1));
// If we set the working bank, the node should be leader within next 2 slots
poh_recorder.set_bank(bank.clone(), false);
poh_recorder.set_bank_for_test(bank.clone());
assert!(poh_recorder.would_be_leader(2 * bank.ticks_per_slot()));
}
}
Expand Down Expand Up @@ -2160,7 +2170,7 @@ mod tests {
for _ in 0..(bank.ticks_per_slot() * 3) {
poh_recorder.tick();
}
poh_recorder.set_bank(bank.clone(), false);
poh_recorder.set_bank_for_test(bank.clone());
assert!(!bank.is_hash_valid_for_age(&genesis_hash, 0));
assert!(bank.is_hash_valid_for_age(&genesis_hash, 1));
}
Expand Down
2 changes: 1 addition & 1 deletion poh/src/poh_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ mod tests {
hashes_per_batch,
record_receiver,
);
poh_recorder.write().unwrap().set_bank(bank, false);
poh_recorder.write().unwrap().set_bank_for_test(bank);

// get some events
let mut hashes = 0;
Expand Down
1 change: 1 addition & 0 deletions scripts/check-dev-context-only-utils.sh
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ source ci/rust-version.sh nightly
# reason to bend dev-context-only-utils's original intention and that listed
# package isn't part of released binaries.
declare tainted_packages=(
solana-banking-bench
solana-ledger-tool
)

Expand Down