From 809041b1519ebaa54d6ffe4ce194f04feb8eda0f Mon Sep 17 00:00:00 2001 From: Illia Bobyr Date: Wed, 22 Mar 2023 11:03:30 -0700 Subject: [PATCH] poh_verify => run_verification: Rename to be more accurate (#30811) `poh_verify` actually disables transaction signature, tick count and built in program argument verifications as well. It is somewhat confusing to call it `poh_verify`. --- core/src/validator.rs | 8 ++++--- ledger-tool/src/main.rs | 32 ++++++++++++++++++------- ledger/src/blockstore_processor.rs | 33 +++++++++++++------------- local-cluster/src/validator_configs.rs | 2 +- local-cluster/tests/local_cluster.rs | 4 ++-- test-validator/src/lib.rs | 2 +- validator/src/cli.rs | 11 ++++++++- validator/src/main.rs | 7 +++++- 8 files changed, 66 insertions(+), 33 deletions(-) diff --git a/core/src/validator.rs b/core/src/validator.rs index 9ddfc1ab48..92deea6b7c 100644 --- a/core/src/validator.rs +++ b/core/src/validator.rs @@ -152,7 +152,9 @@ pub struct ValidatorConfig { pub accounts_hash_interval_slots: u64, pub max_genesis_archive_unpacked_size: u64, pub wal_recovery_mode: Option, - pub poh_verify: bool, // Perform PoH verification during blockstore processing at boo + /// Run PoH, transaction signature and other transaction verifications during blockstore + /// processing. + pub run_verification: bool, pub require_tower: bool, pub tower_storage: Arc, pub debug_keys: Option>>, @@ -216,7 +218,7 @@ impl Default for ValidatorConfig { accounts_hash_interval_slots: std::u64::MAX, max_genesis_archive_unpacked_size: MAX_GENESIS_ARCHIVE_UNPACKED_SIZE, wal_recovery_mode: None, - poh_verify: true, + run_verification: true, require_tower: false, tower_storage: Arc::new(crate::tower_storage::NullTowerStorage::default()), debug_keys: None, @@ -1483,7 +1485,7 @@ fn load_blockstore( .or_else(|| blockstore.highest_slot().unwrap_or(None)); let process_options = blockstore_processor::ProcessOptions { - poh_verify: config.poh_verify, + run_verification: config.run_verification, halt_at_slot, new_hard_forks: config.new_hard_forks.clone(), debug_keys: config.debug_keys.clone(), diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index 73f8a02fca..19ad0babd0 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -1931,7 +1931,16 @@ fn main() { Arg::with_name("skip_poh_verify") .long("skip-poh-verify") .takes_value(false) - .help("Skip ledger PoH verification"), + .help( + "Deprecated, please use --skip-verification.\n\ + Skip ledger PoH and transaction verification." + ), + ) + .arg( + Arg::with_name("skip_verification") + .long("skip-verification") + .takes_value(false) + .help("Skip ledger PoH and transaction verification."), ) .arg( Arg::with_name("print_accounts_stats") @@ -2529,7 +2538,7 @@ fn main() { let process_options = ProcessOptions { new_hard_forks: hardforks_of(arg_matches, "hard_forks"), halt_at_slot: Some(0), - poh_verify: false, + run_verification: false, ..ProcessOptions::default() }; let genesis_config = open_genesis_config_by(&ledger_path, arg_matches); @@ -2620,7 +2629,7 @@ fn main() { let process_options = ProcessOptions { new_hard_forks: hardforks_of(arg_matches, "hard_forks"), halt_at_slot: value_t!(arg_matches, "halt_at_slot", Slot).ok(), - poh_verify: false, + run_verification: false, ..ProcessOptions::default() }; let genesis_config = open_genesis_config_by(&ledger_path, arg_matches); @@ -2828,9 +2837,16 @@ fn main() { let debug_keys = pubkeys_of(arg_matches, "debug_key") .map(|pubkeys| Arc::new(pubkeys.into_iter().collect::>())); + if arg_matches.is_present("skip_poh_verify") { + eprintln!( + "--skip-poh-verify is deprecated. Replace with --skip-verification." + ); + } + let process_options = ProcessOptions { new_hard_forks: hardforks_of(arg_matches, "hard_forks"), - poh_verify: !arg_matches.is_present("skip_poh_verify"), + run_verification: !(arg_matches.is_present("skip_poh_verify") + || arg_matches.is_present("skip_verification")), on_halt_store_hash_raw_data_for_debug: arg_matches .is_present("halt_at_slot_store_hash_raw_data"), // ledger tool verify always runs the accounts hash calc at the end of processing the blockstore @@ -2898,7 +2914,7 @@ fn main() { let process_options = ProcessOptions { new_hard_forks: hardforks_of(arg_matches, "hard_forks"), halt_at_slot: value_t!(arg_matches, "halt_at_slot", Slot).ok(), - poh_verify: false, + run_verification: false, ..ProcessOptions::default() }; @@ -3082,7 +3098,7 @@ fn main() { ProcessOptions { new_hard_forks, halt_at_slot: Some(snapshot_slot), - poh_verify: false, + run_verification: false, accounts_db_config: Some(get_accounts_db_config(&ledger_path, arg_matches)), accounts_db_skip_shrink: arg_matches.is_present("accounts_db_skip_shrink"), ..ProcessOptions::default() @@ -3434,7 +3450,7 @@ fn main() { let process_options = ProcessOptions { new_hard_forks: hardforks_of(arg_matches, "hard_forks"), halt_at_slot, - poh_verify: false, + run_verification: false, ..ProcessOptions::default() }; let genesis_config = open_genesis_config_by(&ledger_path, arg_matches); @@ -3523,7 +3539,7 @@ fn main() { let process_options = ProcessOptions { new_hard_forks: hardforks_of(arg_matches, "hard_forks"), halt_at_slot, - poh_verify: false, + run_verification: false, ..ProcessOptions::default() }; let genesis_config = open_genesis_config_by(&ledger_path, arg_matches); diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index b068ac7b3e..74a872e594 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -595,7 +595,8 @@ pub type ProcessCallback = Arc; #[derive(Default, Clone)] pub struct ProcessOptions { - pub poh_verify: bool, + /// Run PoH, transaction signature and other transaction verifications on the entries. + pub run_verification: bool, pub full_leader_cache: bool, pub halt_at_slot: Option, pub new_hard_forks: Option>, @@ -885,7 +886,7 @@ fn confirm_full_slot( timing: &mut ExecuteTimings, ) -> result::Result<(), BlockstoreProcessorError> { let mut confirmation_timing = ConfirmationTiming::default(); - let skip_verification = !opts.poh_verify; + let skip_verification = !opts.run_verification; let _ignored_prioritization_fee_cache = PrioritizationFeeCache::new(0u64); confirm_slot( @@ -1896,7 +1897,7 @@ pub mod tests { &genesis_config, &blockstore, &ProcessOptions { - poh_verify: true, + run_verification: true, ..ProcessOptions::default() }, blockstore_access_type.clone(), @@ -1951,7 +1952,7 @@ pub mod tests { &genesis_config, &blockstore, &ProcessOptions { - poh_verify: true, + run_verification: true, ..ProcessOptions::default() }, &Arc::default(), @@ -1966,7 +1967,7 @@ pub mod tests { &genesis_config, &blockstore, &ProcessOptions { - poh_verify: true, + run_verification: true, ..ProcessOptions::default() }, &Arc::default(), @@ -2020,7 +2021,7 @@ pub mod tests { ); let opts = ProcessOptions { - poh_verify: true, + run_verification: true, accounts_db_test_hash_calculation: true, ..ProcessOptions::default() }; @@ -2085,7 +2086,7 @@ pub mod tests { fill_blockstore_slot_with_ticks(&blockstore, ticks_per_slot, 2, 1, blockhash); let opts = ProcessOptions { - poh_verify: true, + run_verification: true, accounts_db_test_hash_calculation: true, ..ProcessOptions::default() }; @@ -2103,7 +2104,7 @@ pub mod tests { slot 2 (all ticks) */ let opts = ProcessOptions { - poh_verify: true, + run_verification: true, accounts_db_test_hash_calculation: true, ..ProcessOptions::default() }; @@ -2172,7 +2173,7 @@ pub mod tests { blockstore.set_roots(vec![0, 1, 4].iter()).unwrap(); let opts = ProcessOptions { - poh_verify: true, + run_verification: true, accounts_db_test_hash_calculation: true, ..ProcessOptions::default() }; @@ -2252,7 +2253,7 @@ pub mod tests { blockstore.set_roots(vec![0, 1].iter()).unwrap(); let opts = ProcessOptions { - poh_verify: true, + run_verification: true, accounts_db_test_hash_calculation: true, ..ProcessOptions::default() }; @@ -2466,7 +2467,7 @@ pub mod tests { // Check that we can properly restart the ledger / leader scheduler doesn't fail let opts = ProcessOptions { - poh_verify: true, + run_verification: true, accounts_db_test_hash_calculation: true, ..ProcessOptions::default() }; @@ -2611,7 +2612,7 @@ pub mod tests { ) .unwrap(); let opts = ProcessOptions { - poh_verify: true, + run_verification: true, accounts_db_test_hash_calculation: true, ..ProcessOptions::default() }; @@ -2642,7 +2643,7 @@ pub mod tests { let blockstore = Blockstore::open(ledger_path.path()).unwrap(); let opts = ProcessOptions { - poh_verify: true, + run_verification: true, accounts_db_test_hash_calculation: true, ..ProcessOptions::default() }; @@ -3316,7 +3317,7 @@ pub mod tests { // Specify halting at slot 0 let opts = ProcessOptions { - poh_verify: true, + run_verification: true, halt_at_slot: Some(0), accounts_db_test_hash_calculation: true, ..ProcessOptions::default() @@ -3370,7 +3371,7 @@ pub mod tests { let mut bank_forks = BankForks::new(Bank::new_for_tests(&genesis_config)); let bank0 = bank_forks.get(0).unwrap(); let opts = ProcessOptions { - poh_verify: true, + run_verification: true, accounts_db_test_hash_calculation: true, ..ProcessOptions::default() }; @@ -3834,7 +3835,7 @@ pub mod tests { } let opts = ProcessOptions { - poh_verify: true, + run_verification: true, accounts_db_test_hash_calculation: true, ..ProcessOptions::default() }; diff --git a/local-cluster/src/validator_configs.rs b/local-cluster/src/validator_configs.rs index bcb9783901..2b0b2dcc2c 100644 --- a/local-cluster/src/validator_configs.rs +++ b/local-cluster/src/validator_configs.rs @@ -36,7 +36,7 @@ pub fn safe_clone_config(config: &ValidatorConfig) -> ValidatorConfig { accounts_hash_interval_slots: config.accounts_hash_interval_slots, max_genesis_archive_unpacked_size: config.max_genesis_archive_unpacked_size, wal_recovery_mode: config.wal_recovery_mode.clone(), - poh_verify: config.poh_verify, + run_verification: config.run_verification, require_tower: config.require_tower, tower_storage: config.tower_storage.clone(), debug_keys: config.debug_keys.clone(), diff --git a/local-cluster/tests/local_cluster.rs b/local-cluster/tests/local_cluster.rs index f235caa282..67965725fc 100644 --- a/local-cluster/tests/local_cluster.rs +++ b/local-cluster/tests/local_cluster.rs @@ -23,7 +23,7 @@ use { }, solana_local_cluster::{ cluster::{Cluster, ClusterValidatorInfo}, - cluster_tests::{self}, + cluster_tests, local_cluster::{ClusterConfig, LocalCluster}, validator_configs::*, }, @@ -2154,7 +2154,7 @@ fn create_snapshot_to_hard_fork( let process_options = ProcessOptions { halt_at_slot: Some(snapshot_slot), new_hard_forks: Some(hard_forks), - poh_verify: false, + run_verification: false, ..ProcessOptions::default() }; let ledger_path = blockstore.ledger_path(); diff --git a/test-validator/src/lib.rs b/test-validator/src/lib.rs index fac2949325..33bf9cc045 100644 --- a/test-validator/src/lib.rs +++ b/test-validator/src/lib.rs @@ -941,7 +941,7 @@ impl TestValidator { .unwrap() .0, ], - poh_verify: false, // Skip PoH verification of ledger on startup for speed + run_verification: false, // Skip PoH verification of ledger on startup for speed snapshot_config: SnapshotConfig { full_snapshot_archive_interval_slots: 100, incremental_snapshot_archive_interval_slots: Slot::MAX, diff --git a/validator/src/cli.rs b/validator/src/cli.rs index e26f96a2ff..d2b5521693 100644 --- a/validator/src/cli.rs +++ b/validator/src/cli.rs @@ -613,7 +613,16 @@ pub fn app<'a>(version: &'a str, default_args: &'a DefaultArgs) -> App<'a, 'a> { Arg::with_name("skip_poh_verify") .long("skip-poh-verify") .takes_value(false) - .help("Skip ledger verification at validator bootup"), + .help( + "Deprecated, please use --skip-verification.\n\ + Skip ledger verification at validator bootup." + ), + ) + .arg( + Arg::with_name("skip_verification") + .long("skip-verification") + .takes_value(false) + .help("Skip ledger verification at validator bootup."), ) .arg( Arg::with_name("cuda") diff --git a/validator/src/main.rs b/validator/src/main.rs index 744396f91b..1390c6761d 100644 --- a/validator/src/main.rs +++ b/validator/src/main.rs @@ -1188,6 +1188,10 @@ pub fn main() { } let full_api = matches.is_present("full_rpc_api"); + if matches.is_present("skip_poh_verify") { + eprintln!("--skip-poh-verify is deprecated. Replace with --skip-verification."); + } + let mut validator_config = ValidatorConfig { require_tower: matches.is_present("require_tower"), tower_storage, @@ -1272,7 +1276,8 @@ pub fn main() { repair_whitelist, gossip_validators, wal_recovery_mode, - poh_verify: !matches.is_present("skip_poh_verify"), + run_verification: !(matches.is_present("skip_poh_verify") + || matches.is_present("skip_verification")), debug_keys, contact_debug_interval, send_transaction_service_config: send_transaction_service::Config {