This repository was archived by the owner on Jan 22, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Stabilize some banking stage tests #6251
Merged
carllin
merged 8 commits into
solana-labs:master
from
ryoqun:stabilize-banking-stage-tests
Oct 16, 2019
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
2262ee1
Stabilize some banking stage tests
ryoqun 9df5ba3
Fix CI...
ryoqun b9a248a
clean up
ryoqun cf6d0b0
Fix ci
ryoqun a9138d1
Address review nits
ryoqun b07bf58
Use bank.max_tick_height due to off-by-one for no PohRecord's clearin…
ryoqun 5a39891
Fix CI...
ryoqun cef4770
Use bank.max_tick_height() instead for clarity
ryoqun File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -928,14 +928,15 @@ impl Service for BankingStage { | |
| pub fn create_test_recorder( | ||
| bank: &Arc<Bank>, | ||
| blocktree: &Arc<Blocktree>, | ||
| poh_config: Option<PohConfig>, | ||
| ) -> ( | ||
| Arc<AtomicBool>, | ||
| Arc<Mutex<PohRecorder>>, | ||
| PohService, | ||
| Receiver<WorkingBankEntry>, | ||
| ) { | ||
| let exit = Arc::new(AtomicBool::new(false)); | ||
| let poh_config = Arc::new(PohConfig::default()); | ||
| let poh_config = Arc::new(poh_config.unwrap_or_default()); | ||
| let (mut poh_recorder, entry_receiver) = PohRecorder::new( | ||
| bank.tick_height(), | ||
| bank.last_blockhash(), | ||
|
|
@@ -986,7 +987,7 @@ mod tests { | |
| Blocktree::open(&ledger_path).expect("Expected to be able to open database ledger"), | ||
| ); | ||
| let (exit, poh_recorder, poh_service, _entry_receiever) = | ||
| create_test_recorder(&bank, &blocktree); | ||
| create_test_recorder(&bank, &blocktree, None); | ||
| let cluster_info = ClusterInfo::new_with_invalid_keypair(Node::new_localhost().info); | ||
| let cluster_info = Arc::new(RwLock::new(cluster_info)); | ||
| let banking_stage = BankingStage::new( | ||
|
|
@@ -1011,6 +1012,7 @@ mod tests { | |
| mut genesis_block, .. | ||
| } = create_genesis_block(2); | ||
| genesis_block.ticks_per_slot = 4; | ||
| let num_extra_ticks = 2; | ||
| let bank = Arc::new(Bank::new(&genesis_block)); | ||
| let start_hash = bank.last_blockhash(); | ||
| let (verified_sender, verified_receiver) = unbounded(); | ||
|
|
@@ -1020,8 +1022,10 @@ mod tests { | |
| let blocktree = Arc::new( | ||
| Blocktree::open(&ledger_path).expect("Expected to be able to open database ledger"), | ||
| ); | ||
| let mut poh_config = PohConfig::default(); | ||
| poh_config.target_tick_count = Some(bank.max_tick_height() + num_extra_ticks); | ||
| let (exit, poh_recorder, poh_service, entry_receiver) = | ||
| create_test_recorder(&bank, &blocktree); | ||
| create_test_recorder(&bank, &blocktree, Some(poh_config)); | ||
| let cluster_info = ClusterInfo::new_with_invalid_keypair(Node::new_localhost().info); | ||
| let cluster_info = Arc::new(RwLock::new(cluster_info)); | ||
| let banking_stage = BankingStage::new( | ||
|
|
@@ -1031,7 +1035,6 @@ mod tests { | |
| vote_receiver, | ||
| ); | ||
| trace!("sending bank"); | ||
| sleep(Duration::from_millis(600)); | ||
| drop(verified_sender); | ||
| drop(vote_sender); | ||
| exit.store(true, Ordering::Relaxed); | ||
|
|
@@ -1069,8 +1072,11 @@ mod tests { | |
| let blocktree = Arc::new( | ||
| Blocktree::open(&ledger_path).expect("Expected to be able to open database ledger"), | ||
| ); | ||
| let mut poh_config = PohConfig::default(); | ||
| // limit tick count to avoid clearing working_bank at PohRecord then PohRecorderError(MaxHeightReached) at BankingStage | ||
| poh_config.target_tick_count = Some(bank.max_tick_height() - 1); | ||
| let (exit, poh_recorder, poh_service, entry_receiver) = | ||
| create_test_recorder(&bank, &blocktree); | ||
| create_test_recorder(&bank, &blocktree, Some(poh_config)); | ||
| let cluster_info = ClusterInfo::new_with_invalid_keypair(Node::new_localhost().info); | ||
| let cluster_info = Arc::new(RwLock::new(cluster_info)); | ||
| let banking_stage = BankingStage::new( | ||
|
|
@@ -1120,6 +1126,9 @@ mod tests { | |
|
|
||
| drop(verified_sender); | ||
| drop(vote_sender); | ||
| // wait until banking_stage to finish up all packets | ||
| banking_stage.join().unwrap(); | ||
|
|
||
| exit.store(true, Ordering::Relaxed); | ||
| poh_service.join().unwrap(); | ||
| drop(poh_recorder); | ||
|
|
@@ -1128,18 +1137,20 @@ mod tests { | |
| let bank = Bank::new(&genesis_block); | ||
| bank.process_transaction(&fund_tx).unwrap(); | ||
| //receive entries + ticks | ||
| for _ in 0..10 { | ||
| loop { | ||
| let entries: Vec<Entry> = entry_receiver | ||
| .iter() | ||
| .map(|(_bank, (entry, _tick_height))| entry) | ||
| .collect(); | ||
|
|
||
| assert!(entries.verify(&blockhash)); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this assertion should be moved inside the newly-added empty guard if clause. |
||
| blockhash = entries.last().unwrap().hash; | ||
| for entry in entries { | ||
| bank.process_transactions(&entry.transactions) | ||
| .iter() | ||
| .for_each(|x| assert_eq!(*x, Ok(()))); | ||
| if !entries.is_empty() { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Under high load, I observed empty entries are returned. |
||
| blockhash = entries.last().unwrap().hash; | ||
| for entry in entries { | ||
| bank.process_transactions(&entry.transactions) | ||
| .iter() | ||
| .for_each(|x| assert_eq!(*x, Ok(()))); | ||
| } | ||
| } | ||
|
|
||
| if bank.get_balance(&to) == 1 { | ||
|
|
@@ -1153,13 +1164,11 @@ mod tests { | |
| assert_eq!(bank.get_balance(&to2), 0); | ||
|
|
||
| drop(entry_receiver); | ||
| banking_stage.join().unwrap(); | ||
| } | ||
| Blocktree::destroy(&ledger_path).unwrap(); | ||
| } | ||
|
|
||
| #[test] | ||
| #[ignore] | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yay! |
||
| fn test_banking_stage_entryfication() { | ||
| solana_logger::setup(); | ||
| // In this attack we'll demonstrate that a verifier can interpret the ledger | ||
|
|
@@ -1212,8 +1221,11 @@ mod tests { | |
| Blocktree::open(&ledger_path) | ||
| .expect("Expected to be able to open database ledger"), | ||
| ); | ||
| let mut poh_config = PohConfig::default(); | ||
| // limit tick count to avoid clearing working_bank at PohRecord then PohRecorderError(MaxHeightReached) at BankingStage | ||
| poh_config.target_tick_count = Some(bank.max_tick_height() - 1); | ||
| let (exit, poh_recorder, poh_service, entry_receiver) = | ||
| create_test_recorder(&bank, &blocktree); | ||
| create_test_recorder(&bank, &blocktree, Some(poh_config)); | ||
| let cluster_info = | ||
| ClusterInfo::new_with_invalid_keypair(Node::new_localhost().info); | ||
| let cluster_info = Arc::new(RwLock::new(cluster_info)); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because
poh_config's life time is controlled by thetarget_tick_count, this is no longer needed.