Refactor fault hash injection into lambda (#31093)

* refactor out fault hash inject output AccountsHashVerifier

* refactor faught injector out of AccountHashVerifier

* use type alias

* Apply suggestions from code review

Co-authored-by: Brooks <brooks@prumo.org>

* move type alias

* rename

---------

Co-authored-by: Brooks <brooks@prumo.org>

core/src/accounts_hash_verifier.rs

@@ -38,6 +38,8 @@ use {
     },
 };
 
+pub type AccountsHashFaultInjector = fn(&Hash, Slot) -> Option<Hash>;
+
 pub struct AccountsHashVerifier {
     t_accounts_hash_verifier: JoinHandle<()>,
 }
@@ -51,7 +53,7 @@ impl AccountsHashVerifier {
         cluster_info: &Arc<ClusterInfo>,
         known_validators: Option<HashSet<Pubkey>>,
         halt_on_known_validators_accounts_hash_mismatch: bool,
-        fault_injection_rate_slots: u64,
+        accounts_hash_fault_injector: Option<AccountsHashFaultInjector>,
         snapshot_config: SnapshotConfig,
     ) -> Self {
         // If there are no accounts packages to process, limit how often we re-check
@@ -90,8 +92,8 @@ impl AccountsHashVerifier {
                         snapshot_package_sender.as_ref(),
                         &mut hashes,
                         &exit,
-                        fault_injection_rate_slots,
                         &snapshot_config,
+                        accounts_hash_fault_injector,
                     ));
 
                     datapoint_info!(
@@ -189,8 +191,8 @@ impl AccountsHashVerifier {
         snapshot_package_sender: Option<&Sender<SnapshotPackage>>,
         hashes: &mut Vec<(Slot, Hash)>,
         exit: &Arc<AtomicBool>,
-        fault_injection_rate_slots: u64,
         snapshot_config: &SnapshotConfig,
+        accounts_hash_fault_injector: Option<AccountsHashFaultInjector>,
     ) {
         let accounts_hash = Self::calculate_and_verify_accounts_hash(&accounts_package);
 
@@ -203,8 +205,8 @@ impl AccountsHashVerifier {
             halt_on_known_validator_accounts_hash_mismatch,
             hashes,
             exit,
-            fault_injection_rate_slots,
             accounts_hash,
+            accounts_hash_fault_injector,
         );
 
         Self::submit_for_packaging(
@@ -448,16 +450,6 @@ impl AccountsHashVerifier {
         }
     }
 
-    fn generate_fault_hash(original_hash: &Hash) -> Hash {
-        use {
-            rand::{thread_rng, Rng},
-            solana_sdk::hash::extend_and_hash,
-        };
-
-        let rand = thread_rng().gen_range(0, 10);
-        extend_and_hash(original_hash, &[rand])
-    }
-
     fn push_accounts_hashes_to_cluster(
         accounts_package: &AccountsPackage,
         cluster_info: &ClusterInfo,
@@ -465,19 +457,13 @@ impl AccountsHashVerifier {
         halt_on_known_validator_accounts_hash_mismatch: bool,
         hashes: &mut Vec<(Slot, Hash)>,
         exit: &Arc<AtomicBool>,
-        fault_injection_rate_slots: u64,
         accounts_hash: AccountsHashEnum,
+        accounts_hash_fault_injector: Option<AccountsHashFaultInjector>,
     ) {
-        if fault_injection_rate_slots != 0
+        let hash = accounts_hash_fault_injector
-            && accounts_package.slot % fault_injection_rate_slots == 0
+            .and_then(|f| f(accounts_hash.as_hash(), accounts_package.slot))
-        {
+            .or(Some(*accounts_hash.as_hash()));
-            // For testing, publish an invalid hash to gossip.
+        hashes.push((accounts_package.slot, hash.unwrap()));
-            let fault_hash = Self::generate_fault_hash(accounts_hash.as_hash());
-            warn!("inserting fault at slot: {}", accounts_package.slot);
-            hashes.push((accounts_package.slot, fault_hash));
-        } else {
-            hashes.push((accounts_package.slot, *accounts_hash.as_hash()));
-        }
 
         retain_max_n_elements(hashes, MAX_SNAPSHOT_HASHES);
 
@@ -653,8 +639,8 @@ mod tests {
                 None,
                 &mut hashes,
                 &exit,
-                0,
                 &snapshot_config,
+                None,
             );
 
             // sleep for 1ms to create a newer timestamp for gossip entry

core/src/validator.rs

@@ -3,7 +3,7 @@
 pub use solana_perf::report_target_features;
 use {
     crate::{
-        accounts_hash_verifier::AccountsHashVerifier,
+        accounts_hash_verifier::{AccountsHashFaultInjector, AccountsHashVerifier},
         admin_rpc_post_init::AdminRpcRequestMetadataPostInit,
         banking_trace::{self, BankingTracer},
         broadcast_stage::BroadcastStageType,
@@ -199,7 +199,7 @@ pub struct ValidatorConfig {
     pub repair_whitelist: Arc<RwLock<HashSet<Pubkey>>>, // Empty = repair with all
     pub gossip_validators: Option<HashSet<Pubkey>>, // None = gossip with all
     pub halt_on_known_validators_accounts_hash_mismatch: bool,
-    pub accounts_hash_fault_injection_slots: u64, // 0 = no fault injection
+    pub accounts_hash_fault_injector: Option<AccountsHashFaultInjector>,
     pub accounts_hash_interval_slots: u64,
     pub max_genesis_archive_unpacked_size: u64,
     pub wal_recovery_mode: Option<BlockstoreRecoveryMode>,
@@ -267,7 +267,7 @@ impl Default for ValidatorConfig {
             repair_whitelist: Arc::new(RwLock::new(HashSet::default())),
             gossip_validators: None,
             halt_on_known_validators_accounts_hash_mismatch: false,
-            accounts_hash_fault_injection_slots: 0,
+            accounts_hash_fault_injector: None,
             accounts_hash_interval_slots: std::u64::MAX,
             max_genesis_archive_unpacked_size: MAX_GENESIS_ARCHIVE_UNPACKED_SIZE,
             wal_recovery_mode: None,
@@ -715,7 +715,7 @@ impl Validator {
             &cluster_info,
             config.known_validators.clone(),
             config.halt_on_known_validators_accounts_hash_mismatch,
-            config.accounts_hash_fault_injection_slots,
+            config.accounts_hash_fault_injector,
             config.snapshot_config.clone(),
         );
 

core/tests/epoch_accounts_hash.rs

@@ -199,7 +199,7 @@ impl BackgroundServices {
             &cluster_info,
             None,
             false,
-            0,
+            None,
             snapshot_config.clone(),
         );
 

core/tests/snapshots.rs

@@ -1001,7 +1001,7 @@ fn test_snapshots_with_background_services(
         &cluster_info,
         None,
         false,
-        0,
+        None,
         snapshot_test_config.snapshot_config.clone(),
     );
 

local-cluster/src/validator_configs.rs

@@ -32,8 +32,8 @@ pub fn safe_clone_config(config: &ValidatorConfig) -> ValidatorConfig {
         gossip_validators: config.gossip_validators.clone(),
         halt_on_known_validators_accounts_hash_mismatch: config
             .halt_on_known_validators_accounts_hash_mismatch,
-        accounts_hash_fault_injection_slots: config.accounts_hash_fault_injection_slots,
         accounts_hash_interval_slots: config.accounts_hash_interval_slots,
+        accounts_hash_fault_injector: config.accounts_hash_fault_injector,
         max_genesis_archive_unpacked_size: config.max_genesis_archive_unpacked_size,
         wal_recovery_mode: config.wal_recovery_mode.clone(),
         run_verification: config.run_verification,

local-cluster/tests/local_cluster_slow_2.rs

@@ -4,6 +4,7 @@
 use {
     common::*,
     log::*,
+    rand::{thread_rng, Rng},
     serial_test::serial,
     solana_core::validator::ValidatorConfig,
     solana_gossip::gossip_service::discover_cluster,
@@ -17,6 +18,7 @@ use {
     solana_sdk::{
         client::SyncClient,
         clock::Slot,
+        hash::{extend_and_hash, Hash},
         poh_config::PohConfig,
         signature::{Keypair, Signer},
     },
@@ -74,9 +76,19 @@ fn test_consistency_halt() {
     // Create cluster with a leader producing bad snapshot hashes.
     let mut leader_snapshot_test_config =
         setup_snapshot_validator_config(snapshot_interval_slots, num_account_paths);
+
+    // Prepare fault hash injection for testing.
     leader_snapshot_test_config
         .validator_config
-        .accounts_hash_fault_injection_slots = 40;
+        .accounts_hash_fault_injector = Some(|hash: &Hash, slot: Slot| {
+        const FAULT_INJECTION_RATE_SLOTS: u64 = 40; // Inject a fault hash every 40 slots
+        (slot % FAULT_INJECTION_RATE_SLOTS == 0).then(|| {
+            let rand = thread_rng().gen_range(0, 10);
+            let fault_hash = extend_and_hash(hash, &[rand]);
+            warn!("inserting fault at slot: {}", slot);
+            fault_hash
+        })
+    });
 
     let validator_stake = DEFAULT_NODE_STAKE;
     let mut config = ClusterConfig {