From 60efe309111089abafd49ee2c5c57d74c7630a90 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 6 Nov 2019 01:50:25 -0800 Subject: [PATCH] Allow voting on empty banks (#6719) (#6761) automerge --- core/src/replay_stage.rs | 8 ++-- ledger-tool/src/main.rs | 2 +- local_cluster/src/tests/local_cluster.rs | 46 ++++++++++++++++++-- runtime/src/bank.rs | 53 +++++------------------- 4 files changed, 57 insertions(+), 52 deletions(-) diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index 0ed76e5c9d..c88b5d47f5 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -479,6 +479,9 @@ impl ReplayStage { where T: 'static + KeypairUtil + Send + Sync, { + if bank.is_empty() { + inc_new_counter_info!("replay_stage-voted_empty_bank", 1); + } trace!("handle votable bank {}", bank.slot()); let (vote, tower_index) = tower.new_vote_from_bank(bank, vote_account); if let Some(new_root) = tower.record_bank_vote(vote) { @@ -638,11 +641,6 @@ impl ReplayStage { trace!("frozen_banks {}", frozen_banks.len()); let mut votable: Vec<(u128, Arc, HashMap, u64)> = frozen_banks .values() - .filter(|b| { - let is_votable = b.is_votable(); - trace!("bank is votable: {} {}", b.slot(), is_votable); - is_votable - }) .filter(|b| { let has_voted = tower.has_voted(b.slot()); trace!("bank has_voted: {} {}", b.slot(), has_voted); diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index 92a13eb808..a0d0a17088 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -149,7 +149,7 @@ fn graph_forks( "".to_string() }, if first { "filled," } else { "" }, - if !bank.is_votable() { "dotted," } else { "" } + "" )); styled_slots.insert(bank.slot()); } diff --git a/local_cluster/src/tests/local_cluster.rs b/local_cluster/src/tests/local_cluster.rs index eab715f1aa..9ad51570cc 100644 --- a/local_cluster/src/tests/local_cluster.rs +++ b/local_cluster/src/tests/local_cluster.rs @@ -7,8 +7,8 @@ use log::*; use serial_test_derive::serial; use solana_client::thin_client::create_client; use solana_core::{ - broadcast_stage::BroadcastStageType, gossip_service::discover_cluster, - validator::ValidatorConfig, + broadcast_stage::BroadcastStageType, consensus::VOTE_THRESHOLD_DEPTH, + gossip_service::discover_cluster, validator::ValidatorConfig, }; use solana_ledger::{bank_forks::SnapshotConfig, blocktree::Blocktree, snapshot_utils}; use solana_runtime::accounts_db::AccountsDB; @@ -626,7 +626,47 @@ fn test_faulty_node(faulty_node_type: BroadcastStageType) { } #[test] -#[serial] +// Test that when a leader is leader for banks B_i..B_{i+n}, and B_i is not +// votable, then B_{i+1} still chains to B_i +fn test_no_voting() { + solana_logger::setup(); + let mut validator_config = ValidatorConfig::default(); + validator_config.rpc_config.enable_validator_exit = true; + validator_config.voting_disabled = true; + let config = ClusterConfig { + cluster_lamports: 10_000, + node_stakes: vec![100], + validator_configs: vec![validator_config.clone()], + ..ClusterConfig::default() + }; + let mut cluster = LocalCluster::new(&config); + let client = cluster + .get_validator_client(&cluster.entry_point_info.id) + .unwrap(); + loop { + let last_slot = client.get_slot().expect("Couldn't get slot"); + if last_slot > 4 * VOTE_THRESHOLD_DEPTH as u64 { + break; + } + sleep(Duration::from_secs(1)); + } + + cluster.close_preserve_ledgers(); + let leader_pubkey = cluster.entry_point_info.id; + let ledger_path = cluster.validator_infos[&leader_pubkey] + .info + .ledger_path + .clone(); + let ledger = Blocktree::open(&ledger_path).unwrap(); + for i in 0..2 * VOTE_THRESHOLD_DEPTH { + let meta = ledger.meta(i as u64).unwrap().unwrap(); + let parent = meta.parent_slot; + let expected_parent = i.saturating_sub(1); + assert_eq!(parent, expected_parent as u64); + } +} + +#[test] fn test_repairman_catchup() { solana_logger::setup(); error!("test_repairman_catchup"); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 6e19585c83..9cf450efb5 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -677,9 +677,6 @@ impl Bank { // / ticks/slot / self.ticks_per_slot as f64; - // make bank 0 votable - self.is_delta.store(true, Ordering::Relaxed); - self.epoch_schedule = genesis_block.epoch_schedule; self.inflation = genesis_block.inflation; @@ -1546,8 +1543,8 @@ impl Bank { self.epoch_schedule.get_epoch_and_slot_index(slot) } - pub fn is_votable(&self) -> bool { - self.is_delta.load(Ordering::Relaxed) && self.tick_height() == self.max_tick_height + pub fn is_empty(&self) -> bool { + !self.is_delta.load(Ordering::Relaxed) } /// Add an instruction processor to intercept instructions before the dynamic loader. @@ -1643,7 +1640,6 @@ mod tests { clock::DEFAULT_TICKS_PER_SLOT, epoch_schedule::MINIMUM_SLOTS_PER_EPOCH, genesis_block::create_genesis_block, - hash, instruction::InstructionError, poh_config::PohConfig, rent_calculator::RentCalculator, @@ -2928,38 +2924,19 @@ mod tests { } #[test] - fn test_is_votable() { - // test normal case + fn test_is_empty() { let (genesis_block, mint_keypair) = create_genesis_block(500); - let bank = Arc::new(Bank::new(&genesis_block)); + let bank0 = Arc::new(Bank::new(&genesis_block)); let key1 = Keypair::new(); - assert_eq!(bank.is_votable(), false); - // Set is_delta to true + // The zeroth bank is empty becasue there are no transactions + assert_eq!(bank0.is_empty(), true); + + // Set is_delta to true, bank is no longer empty let tx_transfer_mint_to_1 = system_transaction::transfer(&mint_keypair, &key1.pubkey(), 1, genesis_block.hash()); - assert_eq!(bank.process_transaction(&tx_transfer_mint_to_1), Ok(())); - assert_eq!(bank.is_votable(), false); - - // Register enough ticks to hit max tick height - for i in 0..genesis_block.ticks_per_slot { - bank.register_tick(&hash::hash(format!("hello world {}", i).as_bytes())); - } - - assert_eq!(bank.is_votable(), true); - - // test empty bank with ticks - let (genesis_block, _mint_keypair) = create_genesis_block(500); - // make an empty bank at slot 1 - let bank = new_from_parent(&Arc::new(Bank::new(&genesis_block))); - assert_eq!(bank.is_votable(), false); - - // Register enough ticks to hit max tick height - for i in 0..genesis_block.ticks_per_slot { - bank.register_tick(&hash::hash(format!("hello world {}", i).as_bytes())); - } - // empty banks aren't votable even at max tick height - assert_eq!(bank.is_votable(), false); + assert_eq!(bank0.process_transaction(&tx_transfer_mint_to_1), Ok(())); + assert_eq!(bank0.is_empty(), false); } #[test] @@ -3061,16 +3038,6 @@ mod tests { assert_eq!(vote_accounts.len(), 1); } - #[test] - fn test_bank_0_votable() { - let (genesis_block, _) = create_genesis_block(500); - let bank = Arc::new(Bank::new(&genesis_block)); - //set tick height to max - let max_tick_height = (bank.slot + 1) * bank.ticks_per_slot; - bank.tick_height.store(max_tick_height, Ordering::Relaxed); - assert!(bank.is_votable()); - } - #[test] fn test_bank_fees_account() { let (mut genesis_block, _) = create_genesis_block(500);